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

NoUnneededFinalMethodFixer - Mark as risky #4787

Merged
merged 1 commit into from
Feb 11, 2020
Merged

NoUnneededFinalMethodFixer - Mark as risky #4787

merged 1 commit into from
Feb 11, 2020

Conversation

SpacePossum
Copy link
Contributor

@SpacePossum SpacePossum commented Feb 7, 2020 •
edited by julienfalque

closes #4771

@julienfalque julienfalque changed the title NoUnneededFinalMethodFixer - add option to disable fixing final priva… NoUnneededFinalMethodFixer - Add "private_methods" option Feb 8, 2020
julienfalque
julienfalque reviewed Feb 8, 2020
View reviewed changes
Copy link
Member

@julienfalque julienfalque left a comment

Choose a reason for hiding this comment

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

Looking good but should target master (as new feature).

README.rst Show resolved Hide resolved
@SpacePossum
Copy link
Contributor Author

is there any technical or BC concern for not sending this to 2.15 though?

@julienfalque
Copy link
Member

No, this is a SemVer requirement: backward compatible new features should be added in minor releases, not patch ones.

@SpacePossum
Copy link
Contributor Author

SpacePossum commented Feb 10, 2020 •
edited

marked the fixer as risky, see https://3v4l.org/feUVd
I prefer to merge all in one go on 2.15 and be practical. This saves the maintainers/members time when merging, both for the upcoming releases and in the future as the differences between the branches stays smaller.

@SpacePossum SpacePossum added this to the 2.15.6 milestone Feb 11, 2020
@SpacePossum SpacePossum added the RTM Ready To Merge label Feb 11, 2020
@julienfalque
Copy link
Member

👍 but I still believe adding a new option should only happen on master though.

@SpacePossum
Copy link
Contributor Author

done

@SpacePossum SpacePossum changed the title NoUnneededFinalMethodFixer - Add "private_methods" option NoUnneededFinalMethodFixer - Mark as risky Feb 11, 2020
@SpacePossum
Copy link
Contributor Author

SpacePossum commented Feb 11, 2020 •
edited

phpmd in analyzing the fixtures;

The command "php -d auto_prepend_file=dev-tools/vendor/autoload.php ./dev-tools/tools/phpstan analyse" exited with 0.
$ if [ -n "$CHANGED_PHP_FILES" ]; then ./dev-tools/vendor/bin/phpmd `echo "$CHANGED_PHP_FILES" | xargs | sed 's/ /,/g'` text phpmd.xml || travis_terminate 1; fi
/home/travis/build/FriendsOfPHP/PHP-CS-Fixer/tests/Fixtures/Integration/set/@Symfony_whitespaces.test-out.php:110	The class finalClass is not named in CamelCase.

this is not what we want I hope?

SpacePossum added a commit that referenced this pull request Feb 11, 2020
@SpacePossum
bug #4787 NoUnneededFinalMethodFixer - Mark as risky (SpacePossum)
a836483 
This PR was merged into the 2.15 branch.

Discussion
----------

NoUnneededFinalMethodFixer - Mark as risky

closes #4771

Commits
-------

e9e586f NoUnneededFinalMethodFixer - mark as risky
@SpacePossum SpacePossum merged commit e9e586f into PHP-CS-Fixer:2.15 Feb 11, 2020
@SpacePossum SpacePossum removed the RTM Ready To Merge label Feb 11, 2020
@SpacePossum SpacePossum deleted the 2_15_4771_NoUnneededFinalMethodFixer branch February 11, 2020 18:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@julienfalque julienfalque julienfalque approved these changes

Assignees
No one assigned
Labels
kind/enhancement
Projects
None yet
Milestone
2.15.6
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@SpacePossum @julienfalque