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

Fix setDeprecated deprecation #379

Merged
merged 1 commit into from
Apr 15, 2022
Merged

Fix setDeprecated deprecation #379

merged 1 commit into from
Apr 15, 2022

Conversation

dannyvw
Copy link
Contributor

@dannyvw dannyvw commented Mar 12, 2022

Q A
Bug fix? no
New feature? no
BC breaks? no
Deprecations? yes
Related tickets partially #335
License MIT

@dannyvw dannyvw requested a review from a team as a code owner March 12, 2022 17:07
lchrusciel
lchrusciel reviewed Apr 15, 2022
View reviewed changes
src/Bundle/DependencyInjection/Compiler/PagerfantaBridgePass.php
{
$message = 'The "%alias_id%" service alias is deprecated since Sylius 1.8, use the "pagerfanta.view_factory" service ID instead.';

if (method_exists(Alias::class, 'getDeprecation')) {
Copy link
Member

Choose a reason for hiding this comment

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

Won't it be better to base it on Symfony version? Also, can we see, that it really makes a change in some CI?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The getDeprecation method is added in the same PR, see symfony/symfony#35778. So i think this should be good enough.

Copy link
Member

Choose a reason for hiding this comment

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

I get it, I have the problem, that checking the number of arguments or Symfony version is a more explicit check for me. But perhaps, I'm just picky

@lchrusciel lchrusciel changed the base branch from master to 1.10 April 15, 2022 11:31
@lchrusciel lchrusciel added Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Maintenance Configurations, READMEs, releases, etc. labels Apr 15, 2022
@lchrusciel lchrusciel merged commit 2f6c79e into Sylius:1.10 Apr 15, 2022
@lchrusciel
Copy link
Member

Thank you, Danny! 🥇

@dannyvw dannyvw deleted the alias-deprecation branch April 15, 2022 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lchrusciel lchrusciel lchrusciel left review comments

@vvasiloi vvasiloi vvasiloi approved these changes

Assignees
No one assigned
Labels
Enhancement Minor issues and PRs improving the current solutions (optimizations, typo fixes, etc.). Maintenance Configurations, READMEs, releases, etc.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dannyvw @lchrusciel @vvasiloi