-
Notifications
You must be signed in to change notification settings - Fork 16
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
Check TODO and @todo comments looking for extra info #91
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #91 +/- ##
============================================
+ Coverage 96.46% 96.59% +0.12%
- Complexity 517 537 +20
============================================
Files 22 23 +1
Lines 1471 1525 +54
============================================
+ Hits 1419 1473 +54
Misses 52 52 ☔ View full report in Codecov by Sentry. |
3473450
to
82364c5
Compare
I've created these 2 issues (CiBoT and moodle-plugin-ci) to avoid this new sniff to be executed for plugins, it's only for core: |
d6d7064
to
2b3e35e
Compare
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.
OK, I agree to keep MDL default as nomal use case. Thanks for prompt solution for this issue Eloy!
Ok, please don't merge this yet. We need to change this Sniff That way other tools can use Ciao :-) |
Ok, I've added a new, 3rd commit, that adds the possibility of using a config setting to specify the desired regex to be used.
|
88f25b1
to
633ed49
Compare
This will look for some extra information to exist in: - Inline TODO comments: // TODO blah, blah. - PHPDoc @todo comments: /** @todo blah, blah. */ By default it will look for all TODOs having some 'MDL-[0-9]+' (regular expression), but it can be customised, in phpcs.xml file or in CLI execution by setting the "commentRequiredRegex" property to any alternative regular expression. Setting it to empty string will disable the sniff at all effects, useful for non-core stuff like plugins. Fixes moodlehq#90
While everything continues working the same and the Sniff property (`commentRequiredRegex`) still is available and defaulting to `MDL-[0-9]+`, now we can, also, from command line, decide which regular expression to use. That will make things easier from other tools, not having to modify the ruleset phpcs.xml files. Instead just run: vendor/bin/phpcs --runtime-set moodleTodoCommentRegex 'CONTRIB-[0-9]+' or any other valid regular expression (https, github urls...) In order to disable the Sniff, you can pass to it an empty string (apart from excluding it completely if desired, of course) vendor/bin/phpcs --runtime-set moodleTodoCommentRegex ''
633ed49
to
644bdd2
Compare
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.
Thanks a lot @stronk7 super!
Thanks @paulholden and @kabalin ! |
This will look for some extra information to exist in:
By default it will look for all TODOs having some 'MDL-[0-9]+' (regular expression), but it can be customised, in ruleset , phpcs.xml file or in CLI execution by setting the
$commentRequiredRegex
property or themoodleTodoCommentRegex
configuration option to any alternative regular expression.Setting it to empty string will disable the sniff at all effects, useful for non-core stuff like plugins.
Fixes #90