Problem/Motivation

The method Drupal\Core\Database\Connection::makeSequenceName() is only used internally the PgSQL driver, but it's exposed as a public method for the entire Database API.

Proposed resolution

  1. deprecate Drupal\Core\Database\Connection::makeSequenceName()
  2. mark the PgSql driver implementation @internal or even better a protected method of PgSql's Schema class.

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3221101

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

mondrake created an issue. See original summary.

daffie’s picture

daffie commented

I think the method should be marked as internal as it is being called from the Connection and the Schema class.

Edit: The call from the Connection class is in the to be deprecated method nextId(). Let do a protected method in the Schema class.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

mondrake’s picture

mondrake
Location 🇮🇹
commented

mondrake opened merge request !4467

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Active » Needs review

Deprecated Drupal\Core\Database\Connection::makeSequenceName() which has no longer purpose with ::nextId() being deprecated too. It's needed by the PostgreSql driver, so added a method specific to pgsql's Connection. That needs to be public because other classes within the pgsql driver need to call it.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented

Why do we need to rename the method? We could just copy it to the Postgres driver and trigger a deprecation for it in the base class?

daffie’s picture

daffie commented
Issue tags: +Needs change record
mondrake’s picture

mondrake
Location 🇮🇹
commented

Re. CR, how about just making an addition to https://www.drupal.org/node/3349345 ?

daffie’s picture

daffie commented
Status: Needs review » Needs work

@mondrake: You can make an addition to that CR or create a new one. I think we should also have deprecation message test.

mondrake’s picture

mondrake
Location 🇮🇹
commented

Added draft CR https://www.drupal.org/node/3377046

mondrake’s picture

mondrake
Location 🇮🇹
commented
Status: Needs work » Needs review
Issue tags: -Needs change record
daffie’s picture

daffie commented
Status: Needs review » Reviewed & tested by the community

All my points have been addressed.
All code changes look good to me.
The IS and the CR are in order.
For me it is RTBC.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Reviewed & tested by the community » Fixed

Committed 4365ab3 and pushed to 11.x. Thanks!

Also published the change record.

  • longwave committed 4365ab3f on 11.x 
    Issue #3221101 by mondrake, daffie, longwave: Deprecate Drupal\Core\...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

mondrake closed merge request !4467