Integrate codequality reports into Gitlab

I also found stylelint-formatter-gitlab over in 📌 Add stylelint-junit-formatter to package.json Needs review , happy to merge that issue with this one.

Linking the related feature request for this to be enabled in Contrib templates.

If these were to be installed as part of cores setup it would likely make contrib testing easier too.

Postponed on ✨ [PP-1] Move Gitlab linting steps to main job Active

Ready for review and such again

No substantive review but adding screenshots, what a difference from 'build successful' or 'custom commands failed'.

Updated is.

Alsl added screenshot of the mr overview page, that already shows the cs issues also.

This is fantastic, thanks! Hope to review the code ASAP.

I think we should add the dependencies to composer.json and package.json.

There are some other bits we can clean up now we are not providing junit output.

Removed as suggested.

Dependency evaluation; all these are dev dependencies that do a single small task of converting tool output to a format GitLab can read, none of this is needed at runtime and will in effect only be executed on GitLab.

micheh/phpcs-gitlab

Release cycle: two releases in 2020, none since; both the plugin and the GitLab output format are stable and seemingly require no maintenance.

Security policy: none

Maintainer: Michel Hunziker https://github.com/micheh

eslint-formatter-gitlab

Release cycle: major releases approximately once a year, follows strict semver and updates when NodeJS is updated.

Security policy: none

Maintainer: Remco Haszing https://gitlab.com/remcohaszing

stylelint-formatter-gitlab

Release cycle: three releases in four years

Security policy: none

Maintainer: Leonid Meleshin https://gitlab.com/leon0399

Needs a cspell change but otherwise looks good. Presumably adding this in the main yarn build will help with caching on gitlab a bit too.

Looks great to me. Going to make a massive difference to contributor experience.

Committed/pushed to 11.x, thanks!

This doesn't cherry-pick cleanly to 10.1.x due to the yarn.lock changes - we'll need to decide whether to backport it or have it as 10.2.x-only. Might be better to backport in order to keep the yaml in sync so moving back for now.

Added the dependency evaluation to the issue summary.

Yay it's in!

Backport will be a good idea imo. Not sure what the besta pproach for that would be though.

It caused regression in CI

stylelint and eslint jobs (which are using to override variables) are using PHP 8.1 image

- https://git.drupalcode.org/project/drupal/-/jobs/104537
- https://git.drupalcode.org/project/drupal/-/jobs/104535
vs
- https://git.drupalcode.org/project/drupal/-/jobs/104536
- https://git.drupalcode.org/project/drupal/-/jobs/104538

I mean the following section resetting _TARGET_PHP variable

+  variables:
+    ESLINT_CODE_QUALITY_REPORT: ../eslint-quality-report.json

Correct, it's completely overriding this section:

.default-job-settings: &default-job-settings-lint
  ...
  variables:
    _TARGET_PHP: "8.2"
    _TARGET_DB: "mysql-8"

We could just add ESLINT_CODE_QUALITY_REPORT and STYLELINT_CODE_QUALITY_REPORT to the default-job-settings-lint template and remove those overrides from '🧹 CSS linting (stylelint)': and '🧹 JavaScript linting (eslint)':

I think we should not set those variables at all, and use the default location; the docs for both packages imply the variable is optional.

Opened MR!4840 to try this.

Even better! Keeping an eye on the pipeline. If it works RTBC for the changes from my side.

I thought the code didn't take the working directory into account, but it appears that it does, so trying without the export to see if autodetection does work.

stylelint-quality-report.json: found 1 matching artifact files and directories and eslint-quality-report.json: found 1 matching artifact files and directories and the images are the php-8.2

with the changes in the MR. The only thing now is removing the KUBERNETES_CPU_REQUEST as this will be fixed upstream and is not needed in this MR.

RTBC when that line goes away.

Reverted, this may fail on GitLab CI because of the (unrelated) runner issue.

We can inline, but i've looked at this also in another issue and we should probably decouple the vars. I'll search for my solution

Somethign like:

.default-job-variables: &default-job-variables-lint
  _TARGET_PHP: "8.2"
  _TARGET_DB: "mysql-8"

.default-job-settings: &default-job-settings-lint
  interruptible: true
  allow_failure: false
  variables:
    <<: *default-job-variables-lint
  image:
    name: $_CONFIG_DOCKERHUB_ROOT/php-$_TARGET_PHP-apache:production
  rules:
    - if: $CI_PIPELINE_SOURCE == "push" && $CI_PROJECT_ROOT_NAMESPACE == "project"
    - if: $CI_PIPELINE_SOURCE == "merge_request_event"

  variables:
    <<: *default-job-variables-lint
    STYLELINT_CODE_QUALITY_REPORT: ../stylelint-quality-report.json

That should allow merging normally.

But the quick fix is still just the export inline.

I committed that in 📌 Investigate realistic resource requests for core jobs Active , but haven't run the pipeline yet.

We don't even need the export inline, both packages correctly autodetect the path as seen in the last-but-one successful run.

Yup, RTBC based on that indeed. Lines aren't needed at all.

Committed/pushed to 11.x, thanks! Leaving open for backport again.

10.2.x was branched since this landed, and 10.1.x is security-only, so we can just leave this in 10.2.x

Automatically closed - issue fixed for 2 weeks with no activity.