8000 [Config] Improve the deprecation features by handling package and version by atailouloute · Pull Request #35871 · symfony/symfony · GitHub
[go: up one dir, main page]
Skip to content

[Config] Improve the deprecation features by handling package and version #35871

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
Merged
merged 1 commit into from
Apr 1, 2020
Merged

[Config] Improve the deprecation features by handling package and version #35871

merged 1 commit into from
Apr 1, 2020

Conversation

atailouloute
Copy link
Contributor
@atailouloute atailouloute commented Feb 26, 2020 •
edited by nicolas-grekas
Loading
Q A
Branch? master
Bug fix? no
New feature? yes
Deprecations? yes
Tickets https://github.com/orgs/symfony/projects/1#card-32681032
License MIT
Doc PR TODO

@nicolas-grekas nicolas-grekas added this to the next milestone Feb 26, 2020
stof
stof requested changes Feb 26, 2020
View reviewed changes
Copy link
Member
@stof stof left a comment

Choose a reason for hiding this comment

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

We still need to keep some tests (marked as @group legacy) covering the old API. Otherwise our BC layer is untested.

@atailouloute atailouloute force-pushed the improve-config-deprecation branch 9 times, most recently from 97deb48 to 6f80928 Compare February 26, 2020 16:52
@atailouloute atailouloute requested a review from stof February 26, 2020 19:05
@atailouloute atailouloute force-pushed the improve-config-deprecation branch 2 times, most recently from 603caa7 to 0c829a3 Compare February 26, 2020 19:10
@atailouloute atailouloute force-pushed the improve-config-deprecation branch from 0c829a3 to 5999461 Compare February 26, 2020 20:38
@atailouloute atailouloute force-pushed the improve-config-deprecation branch from 5999461 to 9ebe1a3 Compare February 27, 2020 10:09
nicolas-grekas
nicolas-grekas approved these changes Feb 27, 2020
View reviewed changes
Copy link
Member
@nicolas-grekas nicolas-grekas left a comment

Choose a reason for hiding this comment

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

(with minor comments)

@nicolas-grekas nicolas-grekas mentioned this pull request Feb 27, 2020
Merged
1 task
@atailouloute atailouloute force-pushed the improve-config-deprecation branch 2 times, most recently from 99705c1 to bb1e5bc Compare February 27, 2020 10:41
@atailouloute atailouloute force-pushed the improve-config-deprecation branch from 0fa3233 to c8ddd80 Compare February 27, 2020 14:59
@nicolas-grekas
Copy link
Member

@atailouloute can you please take my comment into account and rebase meanwhile?

@atailouloute atailouloute force-pushed the improve-config-deprecation branch from c8ddd80 to bc42f71 Compare March 31, 2020 20:57
@atailouloute
Copy link
Contributor Author

@nicolas-grekas Done
Some tests are broken (I don't know why), but they are not related to this issue.

@nicolas-grekas nicolas-grekas force-pushed the improve-config-deprecation branch from bc42f71 to f4de76d Compare April 1, 2020 08:41
@nicolas-grekas
Copy link
Member

Thank you @atailouloute.
8000

@nicolas-grekas nicolas-grekas merged commit 0bec08f into symfony:master Apr 1, 2020
@nicolas-grekas nicolas-grekas mentioned this pull request Apr 1, 2020
Closed
This was referenced Apr 1, 2020
Merged
Closed
@atailouloute atailouloute deleted the improve-config-deprecation branch April 11, 2020 13:42
@nicolas-grekas nicolas-grekas modified the milestones: next, 5.1 May 4, 2020
@fabpot fabpot mentioned this pull request May 5, 2020
Merged
apfelbox
apfelbox reviewed Jun 11, 2020
View reviewed changes
src/Symfony/Component/Config/Definition/BaseNode.php
* You can use %node% and %path% placeholders in your message to display,
* respectively, the node name and its complete path.
*/
public function setDeprecated(?string $message)
public function setDeprecated(?string $package/*, string $version, string $message = 'The child node "%node%" at path "%path%" is deprecated.' */)
Copy link
Contributor

Choose a reason for hiding this comment

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

Adding the parameters at the front makes it impossible to both support 5.1+ and <5.1 as a bundle.

If you use the intended 5.1+ way:
->setDeprecated("my/bundle", "1.0", "this node is deprecated, do sth else")

  • in 5.1+ we will show the message "this node is deprecated, do sth else"
  • in <5.1 we will show the message "my/bundle"

Is there a (non-hacky) way to have the real message in both versions?

@nicolas-grekas @atailouloute

Copy link
Contributor

Choose a reason for hiding this comment

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

I just created #37284 to track this

@apfelbox apfelbox mentioned this pull request Jun 15, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nicolas-grekas nicolas-grekas nicolas-grekas approved these changes

@apfelbox apfelbox apfelbox left review comments

@maxhelias maxhelias maxhelias approved these changes

@stof stof Awaiting requested review from stof

Assignees
No one assigned
Labels
Config Deprecation Feature Status: Reviewed
Projects
None yet
Milestone
5.1
Development

Successfully merging this pull request may close these issues.
6 participants
@atailouloute @nicolas-grekas @stof @apfelbox @maxhelias @carsonbot
0