Skip to content

[DoctrineBridge] Improve deprecation when mapping by {id} #57211

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

Closed
wants to merge 1 commit into from

Conversation

nicolas-grekas
Copy link
Member

Q A
Branch? 7.1
Bug fix? no
New feature? no
Deprecations? no
Issues -
License MIT

Follows #54455 (comment)

While the suggestion there is to remove the deprecation altogether, I think we'd better keep it so that we move people to a code that is always mapping explicit. Mapping by exact argument name is supported since the introduction of the entity-value-resolver so its very possible to upgrade existing code while still supporting previous versions of Symfony.

/cc @kevinpapst

@kevinpapst
Copy link

kevinpapst commented May 28, 2024

I am pretty certain that fixing this will cost hundreds of dev hours within all the SF projects out there. That is part of the DX! Sometimes I would love to understand how much the framework wins by deprecating such a code path.

Anyway: finding all places in all templates and dynamically generated routes is a pain on its own, but an argument that I would like to offer are "bookmarks". I have seen it often, that routes and their parameter {id: 6} can be stored in the database. Updating those is way more complicated.

While the suggestion there is to remove the deprecation altogether, I think we'd better keep it so that we move people to a code that is always mapping explicit.

I agree that updating the docs to teach the correct way is great. But let me tell you about a developer who uses SF since years and who is been using {id} as a route parameter for all these years. Not because he had the idea himself.... I was simply following documentation examples and got used to it. And honestly, I have seen enough SF projects to know that I am not the only one dumb enough to fall for the docs.

Mapping by exact argument name is supported since the introduction of the entity-value-resolver so its very possible to upgrade existing code while still supporting previous versions of Symfony.

You know exactly that we have to adjust the code, it is not an option, but a mandatory requirement if we want to stay in the loop with upgrades.

@nicolas-grekas
Copy link
Member Author

OK, after talking with a few others on Slack, let's go to remove this deprecation.

@nicolas-grekas nicolas-grekas deleted the db-tweak branch May 30, 2024 17:58
fabpot added a commit that referenced this pull request May 30, 2024
…ities (nicolas-grekas)

This PR was merged into the 7.1 branch.

Discussion
----------

[DoctrineBridge] Revert deprecating by-{id} mapping of entities

| Q             | A
| ------------- | ---
| Branch?       | 7.1
| Bug fix?      | yes
| New feature?  | no
| Deprecations? | no
| Issues        | -
| License       | MIT

As discussed in #57211
Introduced in #54455

Commits
-------

b81a62c [DoctrineBridge] Revert deprecating by-{id} mapping of entities
@kevinpapst
Copy link

I should be more on Slack I guess.

And sorry for being stubborn. Often I am not sure if I can transport my thoughts clearly.
In such a case I think "okay, I will work on this a few hours (including testing) on one project. I have at least 4 of these projects. And there are a multiple thousand Symfony devs out there. => Whoa, that is a lot of time that has to be invested."

Anyway: thanks for giving it another thought and for listening to everyones arguments 👍

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.

3 participants