moodlehq/moodle-plugin-ci

Add support for limiting scope of grunt

PhMemmel opened this issue · 9 comments

PhMemmel commented

Use case:
When additional plugins needs to be installed by moodle-plugin-ci because the plugin I want to run the pipeline for depends on some other plugins, I want to run grunt and all other jobs exclusively for my plugin, NOT for the dependency plugins.

Currently however, grunt for example checks all installed plugins which means also the installed plugins just needed as dependency.

There need to be a config option (or even change the default behavior) which limits the scope to the plugin itself, not the dependency plugins.

PhMemmel commented

As I have not received further feedback on this I decided to create a pull request. IMO the stylelint task should be scoped to the plugin and not to the moodle directory. However, I might overlook something here. Curious to receive any feedback on this topic.

stronk7 commented

Hi @PhMemmel,

I've been looking to #278 and I'm not convinced that's the way to go, it's perfectly vald to test N plugins together (if they are inter-dependent or whatever).

I've been thinking about this a little bit and wanted to share if something like this would work:

  • Basically, this is about to ensure that the standard way that we have to ignore files and directories also works for the grunt command.
  • To implement it what we could do is to collect all the involved ignores, namely:
    • IGNORE_PATHS
    • IGNORE_NAMES
    • GRUNT_IGNORE_PATHS
    • GRUNT_IGNORE_NAMES
  • And "inject" them into the .stylelintignore and .eslintignore files
  • Then run the command, the files will be ignored without problem.
  • Finally, restitute the original ignore files, so other commands or code that may be using them is not affected.

While it's a little complex to achieve, it has the advantage of using the "standard" way to ignore files and dirs, can be documented and will become better if we support more commands with it (phpunit is still another one that doesn't support it, see #133 ).

How does that sound for you?

Ciao :-)

PhMemmel commented

Hi @stronk7,

thank you for having a look and sharing your thoughts on this.

While I basically agree that it's perfectly ok that you want moodle-plugin-ci to test multiple plugins, I do not think this is the basic requirement and it's not what you would expect moodle-plugin-ci to do "when just running it on your plugin repository". All other jobs that moodle-plugin-ci is doing are targeting the plugin you are running it for: Unit tests are only run for the plugin, phpcs, ..., even grunt amd for example just uses the plugin directory and does not rebuild all the js files of the other plugins (and yes, why would it supposed to do this, we are running the moodle-plugin-ci for this plugin). Only grunt stylelint currently does this thing of checking other plugins as well which does not feel like it's intended. Also these additional plugins usually are dependencies which you have to add to the pipeline to make your plugin work, so you usually do not even have control of these dependencies and you do not care either. You know your plugin depends on it, but you definitely do not want the moodle-plugin-ci runs of your plugin to be influenced by these libraries.

So I think at least the default should be to only run the stylelint task for the current plugin.

Your suggestions about enhancing grunt to be able to ignore files of course should be pursued as well, but IMO it's kind of another issue.

stronk7 commented

Just linking this with https://tracker.moodle.org/browse/MDL-81263 that would fix this automatically, and in the more natural way, I would say.

stronk7 commented

So I think at least the default should be to only run the stylelint task for the current plugin.

Uhm, let me think about this a little more… maybe you’re ok, but I want to see how all the commands are behaving right now…

Thanks!

PhMemmel commented

Sure, there is a good chance I might misunderstand something here or missing out on an already existing option or sth. Thanks again! :)

BTW: I do not see why https://tracker.moodle.org/browse/MDL-81263 would fix this automatically, but I can see that this could be used to implement something like you suggested. MDL-81263 is about being able to ignore files from the plugin I'm currently running the moodle-plugin-ci for, this issue here however is about ignoring files from OTHER plugins I have to add to the pipeline because of the fact that my plugin is depending on them.

stronk7 commented

Yeah, not automatically, sorry I was too much excited (and having breakfast, aka sleepy), LOL.

I just imagined that people would adopt that (distributed ignore files), to add whatever shouldn't be checked in their plugins and that would, ultimately, lead to the "real" plugin being checked not failing because of dependencies.

But yes, it won't fix this issue by default, just add a new way to avoid problems sometimes.

Ciao :-)

marinaglancy commented

Hi! I was about to report a new issue but found this one.

Here is my use case: I'm trying to make a single version of the plugin that works on all Moodle versions from very ancient (I went back as far as 3.5) as well as 4.3 and 'main'.

The problem I found is that I can not use ES6 in the old Moodle versions, so I have to package two types of JS files - AMD modules and ES6 modules. I have to compile the "AMD" ones separately using older node/grunt.

So basically, I want to run grunt check in moodle-plugin-ci, because it is useful but I want to exclude some JS files.

Let's say I include two files: vault.js and vault_amd.js

  • When running grunt check for MOODLE_403_STABLE I want to exclude vault_amd.js
  • When running grunt check for MOODLE_35_STABLE I want to exclude vault.js
  • For other Moodle versons I will skip grunt check.

P.S. Right now I'm considering the workaround - add a manual step in the GHA file that removes these JS files completely before running moodle-plugin-ci checks