-
-
Notifications
You must be signed in to change notification settings - Fork 57
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Respect all sniffs when reviewing PHP_CodeSniffer itself #122
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@fredden Thanks for porting this over. In its current state (with only one remark), I'm okay to accept this.
Note: further changes to the ruleset used by PHPCS are not my current priority. If anything, I'll likely want to standardize on something closer to PHPCSDevCS at some point in the future.
src/Util/Tokens.php
Outdated
* @link https://www.php.net/language.constants.predefined PHP Manual on magic constants | ||
* @see https://www.php.net/language.constants.predefined PHP Manual on magic constants |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd rather keep this at the correct tag (@link
) than change it just to satisfy an outdated sniff. Could you please add an exception instead ?
P.S.: feel free to squash the commits to one commit when updating the PR. |
1a54c58
to
4569959
Compare
Thanks @fredden ! FYI: I'm not adding a changelog entry as this is not a user-facing change. |
I made a start on this. I got to the point where the standard is installed and runs successfully. The next step involves changing nearly every file in the repository. I've put it down for now. Would a draft pull request be valuable? I can trivially open this as a reminder / TODO item if you'd like. |
Sorry, while I appreciate what you are trying to do, this is definitely not a priority at this moment and not something I want to spend time on at this time. An open issue to discuss a plan to get there is one thing, an open PR, draft or not, will just be closed as noise at this time. |
I agree regarding priorities. I know you have your own way of tracking backlog items; I trust that this is somewhere in that system. For future reference, the work started here includes 09d13e3 and PHPCSStandards/composer-installer#215. |
I will be opening issues for most of those ideas/plans/backlog items, #105, #106, #120 and some older issues ported over from the Squizlabs repo are just the start of that, but 3.8.0 needs to get released first. The rest can wait and is just a distraction at this moment. |
Description
This is a re-creation of squizlabs/PHP_CodeSniffer#3914
While working on another task, I noticed that warnings are ignored by default. This seems unintentional. Some discussion regarding this has taken place in squizlabs/PHP_CodeSniffer#3914
Suggested changelog entry
Respect warnings as well as errors from sniffs within the coding standard for PHP_CodeSniffer itself.
Types of changes
PR checklist