search

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

deprecates integration with PHP Copy/Paste Detector (PHPCPD) library. #264

Closed stopfstedt closed 5 months ago

stopfstedt commented 7 months ago

deprecation as a stepping stone towards full removal.

i tried to be cheeky by triggering a deprecation error when the command is executed, but that didn't pan out. so it's all documentation, and one @deprecated annotation on the command itself.

refs #262 refs #263

codecov[bot] commented 6 months ago

Codecov Report

Attention: 9 lines in your changes are missing coverage. Please review.

Comparison is base (0aef673) 85.10% compared to head (02cb44e) 84.77%.

Files Patch % Lines
src/Command/CopyPasteDetectorCommand.php 18.18% 9 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #264 +/- ## ============================================ - Coverage 85.10% 84.77% -0.34% - Complexity 718 720 +2 ============================================ Files 75 75 Lines 2216 2226 +10 ============================================ + Hits 1886 1887 +1 - Misses 330 339 +9 ```

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

stronk7 commented 5 months ago

Thanks @stopfstedt (and sorry for the late reply, I've been distracted with other stuff).

Your patch looks ok, only detail that, maybe, we could try is to print some warning as an annotation in the workflow. I've not tried that myself, but think that it can be an easy and effective way to inform to everybody about the deprecation (if it works).

Also, I've seen that the CLI.md (that is auto-generated), does include the (DEPRECATED) information. Has that been manually added by you or it's automatic Symfony stuff that adds it for deprecated commands? I can check, heh, but asking is cheaper, TIA!

Ciao :-)

stronk7 commented 5 months ago

Hi again, @stopfstedt.

I’ve taken the liberty of adding one extra commit on top of your changes (squashed to just 1 commit, for clarity).

Here you can see the proposed changes: https://github.com/moodlehq/moodle-plugin-ci/compare/main...stronk7:moodle-plugin-ci:deprecate_copy_paste_detector

And here you can see how it will look for anybody running the command via GitHub workflows, with annotations reminding about the deprecation all the time:

https://github.com/stronk7/moodle-plugin-ci/actions/runs/7775806197

Tell me what do you think and, if you like it, I will add the reworked commits to your branch, so anybody else in the team, can take a look to it.

Ciao 😊

stopfstedt commented 5 months ago

Tell me what do you think and, if you like it, I will add the reworked commits to your branch, so anybody else in the team, can take a look to it.

This is great, please go ahead. Thanks!

stronk7 commented 5 months ago

This is great, please go ahead. Thanks!

To you! I've forced-updated your branch and now it contains the 2 commits described above.

Note (for the reviewer) that it's expected for this change to cause a slight decrease in PHPUnit code coverage, but that is expected and on-purpose, because we don't want the 2 warnings (Symfony and GHA) to be emitted when running tests.

Ciao :-)

stronk7 commented 5 months ago

Thanks @paulholden , merging this now. And thanks too @stopfstedt !