moodlehq / moodle-plugin-ci

Helps running Moodle plugins analysis checks and tests under various CI environments.
https://moodlehq.github.io/moodle-plugin-ci/
GNU General Public License v3.0
41 stars 43 forks source link

Prevent plugin runs to use the moodle.Commenting.TodoComment sniff #267

Closed stronk7 closed 5 months ago

stronk7 commented 5 months ago

A new todo-comment-regex option has been added to the phpcs command.

It allows to specify the regex that will be used with inspecting todo (TODO and @todo) comments.

By default, an empty string is used for the option and that makes the Sniff to stop checking. Whoever wants to check for anything (tracker issue, github issue, arbitrary url, ...) can us the new option to configure it.

Fixes #266

codecov[bot] commented 5 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (8f23cf5) 85.17% compared to head (2b00727) 85.19%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #267 +/- ## ============================================ + Coverage 85.17% 85.19% +0.02% Complexity 718 718 ============================================ Files 75 75 Lines 2232 2236 +4 ============================================ + Hits 1901 1905 +4 Misses 331 331 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

stronk7 commented 5 months ago

Ok, we have agreed to add a new option (ttodo-comment-regex) to the phpcs command.

  1. It will default to '' (or null), so the Sniff won't do anything.
  2. People wanting to enable it just will have to set the option with a valid regexp and then the Sniff will run and report.

So this PR is not valid right now... working on it soon... ciao :-)

stronk7 commented 5 months ago

Commit amended, now the new todo-comment-regextakes the baton to decide how to proceed:

Ciao :-)

kabalin commented 5 months ago

Changelog record is required (feel free to add later).

stronk7 commented 5 months ago

Thanks @kabalin,

I'll add the CHANGELOG details, yep (tomorrow).

Also I've converted this to draft because we need the new moodle-cs release to happen in order to get the new unit tests that I've added passing (they require a working moodle-cs with the new config option).

Once we bump to new moodle-cs, I'll complete this.

Ciao :-)

stronk7 commented 5 months ago

CHANGELOG updated with text:

Added

Still holding on https://github.com/moodlehq/moodle-cs/pull/91 and moodle-cs release, to progress this. Not in a hurry.

stronk7 commented 5 months ago

About to go merging this (once tests end), as far as it was approved with all the changes but the CHANGELOG ones that came later.

Immediately after getting this landed, will make a release, so it can be tested out there (default and custom regular expressions).

Ciao :-)