Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Platform API cleanup #4802

Merged
merged 4 commits into from
Sep 19, 2021
Merged

Platform API cleanup #4802

merged 4 commits into from
Sep 19, 2021

Conversation

morozov
Copy link
Member

@morozov morozov commented Sep 19, 2021

Q A
Type improvement, deprecation
BC Break no

The methods being reworked are defined in the AbstractPlatform sub-classes but do not belong to the Platform API:

  1. OraclePlatform::assertValidIdentifier() is not used anywhere. The DBAL is not the source of truth about the validity of a given identifier.
  2. OraclePlatform::getCreateAutoincrementSql() is only called from within the class and should be made protected.
  3. OraclePlatform::getDropAutoincrementSql() is only called from within the schema manager. There's definitely some asymmetry with the above method. We can decide on how to improve this API later. For instance, get rid of the triggers in favor of identity columns.
  4. The SQLServerPlatform methods in question are only called from within the class and should be made protected.
  5. The SQLite user-defined functions don't need to be defined within the platform class.

@derrabus
Copy link
Member

FIY: The ORM does not use any of those methods.

@morozov morozov merged commit 4221163 into doctrine:3.2.x Sep 19, 2021
@morozov morozov deleted the platform-cleanup branch September 19, 2021 17:32
@morozov morozov mentioned this pull request Sep 30, 2021
1 task
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Sep 20, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants