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

Remove obsolete and unnecessary properties from Table attribute (#11351) #11353

Conversation

DaDeather
Copy link
Contributor

The properties indexes and uniqueConstraints were present before and were used for the AnnotationDriver but were never implemented for the AttributeDriver.
Since the AnnotationDriver doesn't exist anymore these can be safely removed reducing possible confusion when defining indices and uniqueConstraints in the Table attribute which never worked anyway.

Fixes #11351

SenseException
SenseException previously approved these changes Mar 13, 2024
@greg0ire
Copy link
Member

Having doubts regarding whether this is the right target branch for this/whether there should be a deprecation instead. @derrabus , WDYT?

@greg0ire greg0ire requested a review from derrabus March 14, 2024 12:19
@derrabus
Copy link
Member

Indeed, this is a breaking change. We need to deprecate those properties first. Target 3.2.x for the deprecation and 4.0.x for the removal.

@greg0ire greg0ire changed the base branch from 3.1.x to 4.0.x March 14, 2024 14:34
@greg0ire greg0ire dismissed SenseException’s stale review March 14, 2024 14:34

The base branch was changed.

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

Retargeted the branch. Please add a note about this in UPGRADE.md and open a separate PR deprecating both properties.

@DaDeather DaDeather force-pushed the 11351-remove-obsolete-indexes-and-unique-constraint-properties-from-table-attribute branch from b1b6de7 to f71c7f7 Compare March 15, 2024 08:00
@DaDeather
Copy link
Contributor Author

Retargeted the branch. Please add a note about this in UPGRADE.md and open a separate PR deprecating both properties.

Here's the one for the deprecation: #11357

beberlei
beberlei previously approved these changes Mar 16, 2024
SenseException
SenseException previously approved these changes Mar 17, 2024
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.

Indeed, this is a breaking change.

Thanks, I've missed that the target branch wasn't the appropriate one.

Copy link
Member

@greg0ire greg0ire left a comment

Choose a reason for hiding this comment

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

This needs to be rebased, and the deprecation code needs to be removed

@DaDeather DaDeather dismissed stale reviews from SenseException and beberlei via 3dba7fb March 18, 2024 07:28
@DaDeather DaDeather force-pushed the 11351-remove-obsolete-indexes-and-unique-constraint-properties-from-table-attribute branch from 3dba7fb to 07fd9df Compare March 18, 2024 07:30
@DaDeather
Copy link
Contributor Author

This needs to be rebased, and the deprecation code needs to be removed

👍 done!

…ctrine#11351)

The properties `indexes` and `uniqueConstraints` were present before and
were used for the `AnnotationDriver` but were never implemented
for the `AttributeDriver`.
Since the `AnnotationDriver` doesn't exist anymore these can be safely
removed reducing possible confusion when defining indices and
uniqueConstraints in the `Table` attribute which never worked anyway.
@DaDeather DaDeather force-pushed the 11351-remove-obsolete-indexes-and-unique-constraint-properties-from-table-attribute branch from 07fd9df to 9430d56 Compare March 18, 2024 07:31
@greg0ire greg0ire added this to the 4.0.0 milestone Mar 18, 2024
@greg0ire greg0ire merged commit 637cd6e into doctrine:4.0.x Mar 18, 2024
64 checks passed
@greg0ire
Copy link
Member

Congrats on your first contribution @DaDeather !

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.

AttributeDriver ignores index and unique constraint definitions in Table attribute
5 participants