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

Make the schema manager aware of the disabling of type comments #6483

Merged
merged 1 commit into from
Aug 7, 2024

Conversation

stof
Copy link
Member

@stof stof commented Aug 6, 2024

Q A
Type bug
Fixed issues n/a

Summary

This makes the schema manager aware of the disabling of type comments to disable its special handling of the type comments during introspection.

Without this change, the schema comparison will not remove existing type comments when disabling them because it will still silently remove that part of the comment during introspection (and so won't detect an expected change between the expected empty comment and the introspected comment).
Disabling that stripping logic when disabling type comments corresponds to the behavior of DBAL 4 regarding comments, which is exactly what the configuration option is about.

Without this change, the schema comparison will not remove existing type
comments when disabling them because it will still silently remove that
part of the comment during introspection (and so won't detect an
expected change between the expected empty comment and the introspected
comment).
Disabling that stripping logic when disabling type comments corresponds
to the behavior of DBAL 4 regarding comments, which is exactly what the
configuration option is about.
@greg0ire greg0ire added the Bug label Aug 6, 2024
@stof
Copy link
Member Author

stof commented Aug 6, 2024

Before this PR, the configuration setting would work fine in new projects and for new fields in existing projects, but would not clean the DB schema for existing fields (until upgrading to 4.0)

Copy link
Member

@SenseException SenseException left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Failing SQL Server test workflows are unrelated to the changes.

@greg0ire greg0ire closed this Aug 7, 2024
@greg0ire greg0ire reopened this Aug 7, 2024
@greg0ire
Copy link
Member

greg0ire commented Aug 7, 2024

Testing an updated merge commit.

@greg0ire greg0ire merged commit 2093d67 into doctrine:3.8.x Aug 7, 2024
160 of 169 checks passed
@greg0ire greg0ire added this to the 3.8.7 milestone Aug 7, 2024
@greg0ire
Copy link
Member

greg0ire commented Aug 7, 2024

Thanks @stof !

@stof stof deleted the fix_type_comment_removal branch August 7, 2024 12:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants