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.

Sign up for GitHub

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

Jump to bottom

Support options like charset and collation on DiscriminatedColumn #10601

Merged
merged 1 commit into from
Mar 30, 2023
Merged

Support options like charset and collation on DiscriminatedColumn #10601

merged 1 commit into from
Mar 30, 2023

Conversation

JanTvrdik
Copy link
Contributor

Closes: #10462

@JanTvrdik JanTvrdik force-pushed the discriminated-column-options branch from 4c7789c to f435baa Compare March 29, 2023 14:11
greg0ire
greg0ire reviewed Mar 29, 2023
View reviewed changes
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 looks like a new feature, and as such should target 2.15.x

lib/Doctrine/ORM/Mapping/Builder/ClassMetadataBuilder.php
*
* @return $this
*/
public function setDiscriminatorColumn($name, $type = 'string', $length = 255, ?string $columnDefinition = null, ?string $enumType = null)
public function setDiscriminatorColumn($name, $type = 'string', $length = 255, ?string $columnDefinition = null, ?string $enumType = null, array $options = [])
Copy link
Member

Choose a reason for hiding this comment

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

Technically a breaking change for extending classes, but we didn't frown upon a similar change in #10288 , so maybe we should just make the class as final on 3.0.x.

Copy link
Member

Choose a reason for hiding this comment

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

BTW, that PR modified docs/en/reference/php-mapping.rst , and this one should too I think. You might want to look at that PR for other changes that need to be done.

@JanTvrdik JanTvrdik force-pushed the discriminated-column-options branch from f435baa to dc3f3f7 Compare March 29, 2023 14:22
@JanTvrdik JanTvrdik changed the base branch from 2.14.x to 2.15.x March 29, 2023 14:22
@JanTvrdik JanTvrdik force-pushed the discriminated-column-options branch from dc3f3f7 to 7f08f33 Compare March 29, 2023 14:51
@JanTvrdik JanTvrdik force-pushed the discriminated-column-options branch 2 times, most recently from 7a53e72 to 2f7ac47 Compare March 30, 2023 09:51
@JanTvrdik JanTvrdik force-pushed the discriminated-column-options branch from 2f7ac47 to f215515 Compare March 30, 2023 09:58
@derrabus derrabus added this to the 2.15.0 milestone Mar 30, 2023
@greg0ire greg0ire merged commit 7ed28db into doctrine:2.15.x Mar 30, 2023
@greg0ire
Copy link
Member

Thanks @JanTvrdik !

@JanTvrdik JanTvrdik deleted the discriminated-column-options branch March 30, 2023 13:56
@mattjanssen mattjanssen mentioned this pull request Feb 15, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@greg0ire greg0ire greg0ire approved these changes

@derrabus derrabus derrabus approved these changes

@SenseException SenseException Awaiting requested review from SenseException

Assignees
No one assigned
Labels
Improvement
Projects
None yet
Milestone
2.15.0
Development

Successfully merging this pull request may close these issues.

Add options (charset, collation) to DiscriminatorColumn to align it with Column options
3 participants
@JanTvrdik @greg0ire @derrabus