-
-
Notifications
You must be signed in to change notification settings - Fork 356
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
[Cleanup] Remove @changelog as no longer maintained/used, use RuleDefinition instead #6035
Conversation
@@ -15,9 +15,6 @@ | |||
use Webmozart\Assert\Assert; | |||
|
|||
/** | |||
* @changelog https://php.watch/versions/8.1/version_compare-operator-restrictions |
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 think @changelog
can be replaced with @see
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.
That's an option. Should be part of rule definition though, otherwise it's only visible when rule is opened (rarely :))
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.
E.g. this link is completelly wrong. It's a different feature.
That's why I want to remove those, as docblocks are not part of rule definition and can be incorrectly copy-pasted.
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.
Just checked couple of those and they're either:
- link to 3v4l.org = those should be in a code example snippet or tests
- links to PHP rfc
- dead links
I'll keep them removed, as I want to keep focused on sets rather than particular rules. Similar way PHPStan does, to make it easy to install Rector and run it 👍
79472f8
to
217e348
Compare
@@ -87,7 +83,7 @@ public function run() | |||
CODE_SAMPLE | |||
, | |||
[ | |||
AddLiteralSeparatorToNumberRector::LIMIT_VALUE => 1_000_000, | |||
self::LIMIT_VALUE => 1_000_000, |
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.
configurable example should use original class reference instead of self::
to ease copy paste config
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.
We've added this feature many years ago, but we no longer maintain, nor use it. There is already rule RuleDefinition that should be used for any explanation.
Time to cleanup the code 👍