Skip to content

fixed argument order for UniqueEntity #19776

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

Merged
merged 1 commit into from
Apr 11, 2024

Conversation

ghost
Copy link

@ghost ghost commented Apr 11, 2024

Q A
Bug fix? no
New feature? no
Issues none
License MIT

Updated parameter order to match order in the constructor to prevent showing a IDE warning.

@ghost ghost requested a review from xabbuh as a code owner April 11, 2024 11:29
@ghost ghost changed the base branch from 7.1 to 5.4 April 11, 2024 11:29
@xabbuh
Copy link
Member

xabbuh commented Apr 11, 2024

We can merge this change as it’s not wrong per se, but I think this should also be reported with your IDE as the order does not matter when using named arguments as we do here.

@ghost
Copy link
Author

ghost commented Apr 11, 2024

I recently had a similar pull request in symfony/ux.
symfony/ux#1703

should i ignore this kind of thing in the future?

@xabbuh xabbuh added this to the 5.4 milestone Apr 11, 2024
@xabbuh
Copy link
Member

xabbuh commented Apr 11, 2024

feel free to still submit this kind of PRs :)

@javiereguiluz
Copy link
Member

This kind of PR are no fun for maintainers because it can introduce conflicts when merging into newer branches ... but it's true that the underlying problem (the IDE complaining about this) is very annoying. So, let's try to fix some of these issues until we decide that it's too much.

Thanks @LtMost!

@javiereguiluz javiereguiluz merged commit 273581b into symfony:5.4 Apr 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants