Only run linting jobs if the files changed make sense for the job

This is a good idea, and just to confirm, the new behavior should be limited to MR branches only. Scheduled and Commit pipelines should run everything, I think.

We also might want a new option variable to turn off this filtering and run all jobs in a MR just like now. There are cases when, say, Coder has introduced a new sniff, or you are working on fixing coding standards that the project ignores. There needs to be a way to override the new behavior and find all places where the new standard fails. Using the scheduled pipeline would not be enough in this scenario as the sniff would be ignored in the committed code phpcs.xml so would not find the failures.

I have updated the issue summary for eslint because we currently also check YML files. That job has

  rules:
    - *skip-eslint-rule
    - exists:
        - "**/*.js"
        - "**/*.yml"

So I think we should leave the skip-...-rule unaltered, but add a - changes: to the actual job. Or maybe the exists: can just become changes:

Could this also be extended to the composer-lint and phpcs jobs? The set of files is larger for those jobs, but the principle is still the same.

the new behavior should be limited to MR branches only. Scheduled and Commit pipelines should run everything, I think

Correct, that's the way I see it too.

We also might want a new option variable to turn off this filtering

Happy with that approach.

Could this also be extended to the composer-lint and phpcs jobs? The set of files is larger for those jobs, but the principle is still the same.

Yup, 100%. I think all linting jobs where it makes sense.

--

It won't be as straightforward as replacing "exists" with "changes" as "changes" is MR specific, but we will start discovering those things once the work on this issue starts.

For example for the eslint job we should match core's behavior:

'🧹 JavaScript linting (eslint)':
…
  # Run on push, or on MRs if CSS files have changed, or manually.
  rules:
    - if: $CI_PIPELINE_SOURCE == "push" && $CI_PROJECT_ROOT_NAMESPACE == "project"
    - if: $CI_PIPELINE_SOURCE == "merge_request_event"
      changes:
        - core/.eslint*
        - core/.prettier*
        - core/package.json
        - core/yarn.lock
        - "**/*.js"
        - "**/*.yml"
    - when: manual
      allow_failure: true

… but adjust for contrib:

  # Run on push, or on MRs if CSS files have changed, or manually.
  rules:
    - if: $CI_PIPELINE_SOURCE == "push" && $CI_PROJECT_ROOT_NAMESPACE == "project"
    - if: $CI_PIPELINE_SOURCE == "merge_request_event"
      changes:
        - "**/.eslint*"
        - "**/.prettier*"
        - "**/package.json"
        - "**/yarn.lock"
        - "**/*.js"
        - "**/*.yml"
    - when: manual
      allow_failure: true

I've implemented this for the contrib module I'm currently working on: #3460778-3: CI: do not run PHP-only jobs if an MR only changes the UI → .

Works well 👍 Speeds up CI runs, reduces CI resource consumption, and improves signal-to-noise ratio of CI.

Crucially:

• Merge request pipelines, rules:changes compares the changes with the target MR branch.
• Branch pipelines, rules:changes compares the changes with the previous commit on the branch.

— https://docs.gitlab.com/ee/ci/yaml/#ruleschanges

I spoke too soon: #3460778-6: CI: do not run PHP-only jobs if an MR only changes the UI → 😭