[JENKINS-68904] Run stylelint fix for less

[NotMyFault]

relates to JENKINS-68887

The change proposed is generated by invoking lint:css:fix via npx, as defined in war/package.json

Keep in mind that this change doesn't address all violations, determining no-descending-specificity for false positives and eslint for js is still up.

Proposed changelog entries

• N/A, skip changelog

Proposed upgrade guidelines

N/A

Submitter checklist

• (If applicable) Jira issue is well described
• Changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developer, depending on the change) and are in the imperative mood. Examples
  • Fill-in the Proposed changelog entries section only if there are breaking changes or other changes which may require extra steps from users during the upgrade
• Appropriate autotests or explanation to why this change has no tests
• New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadoc, as appropriate.
• New deprecations are annotated with @Deprecated(since="TODO") or @Deprecated(since="TODO", forRemoval=true) if applicable.
• For dependency updates: links to external changelogs and, if possible, full diffs

Desired reviewers

@mention

Maintainer checklist

Before the changes are marked as ready-for-merge:

• There are at least 2 approvals for the pull request and no outstanding requests for change
• Conversations in the pull request are over OR it is explicit that a reviewer does not block the change
• Changelog entries in the PR title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood
• Proper changelog labels are set so that the changelog can be generated automatically
• If the change needs additional upgrade steps from users, upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the PR title. (example)
• If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[basil]

See JENKINS-68887

This PR is not a fix for JENKINS-68887, nor does it get us any closer to a fix; while valuable, it is an orthogonal change. The problem described in JENKINS-68887 is that linting war/src/main/less/abstracts/theme.less on Windows causes the linter to crash with "Unknown word CssSyntaxError". The same crash is present in the Windows CI build for this PR, and I see no evidence that the other changes to that file in this PR would meaningfully reduce the chances of hitting a CssSyntaxError.

[timja]

No but it does reduce the linting warnings a lot, is there a reason you haven’t approved this?

it says see jira which is where the context is as a big list of linting warnings was posted there

[basil]

No but it does reduce the linting warnings a lot

Of course, that is why I wrote it was a valuable but orthogonal change.

it says see jira which is where the context is as a big list of linting warnings was posted there

Sure but those warnings were posted as an example of the correct behavior (on Linux), not the incorrect behavior (the fatal error on Windows). I wanted to make sure we were all on the same page that this PR was not a fix for the reported issue but an orthogonal cleanup. Alex updated the description to read "relates to" so I think we are all on the same page.

is there a reason you haven’t approved this?

Well I haven't reviewed the code, nor did I originally plan to. It seems there has been a lack of reviews, though, so I can try and review this later this morning to unblock. In general frontend reviews take some effort for me, as I typically have to educate myself about unfamiliar areas. But I am willing to acquire new skills; I do not give up.

[NotMyFault]

Alex updated the description to read "relates to"

Tim did 👀

Well I haven't reviewed the code, nor did I originally plan to. It seems there has been a lack of reviews

This PR is not urgent or blocks other work, there are enough people on the ux team who can review this PR as well, no worries.

[basil]

Fixes 112 warnings! 👍

The only issue I see is with the fixes for selector-list-comma-newline-after. The automatic fix action put each one on a new line, indenting it by one space. That indentation is wrong: existing precedent in these files shows that each entry should be on a new line, indented by the same amount of space as the first entry. And this is much easier to read as well. I gave a few examples in the suggestions, but I didn't go through all of them. Once that is fixed I am happy to approve this.

[NotMyFault]

There seems to be no format option that does exactly what you want, but with

"selector-list-comma-newline-after": "never-multi-line",
"selector-list-comma-space-after": "always"

we can achieve the following and restore what is already in place and used to be used before, which reads better imo

  &__button, &__icon {
    svg, .build-status-icon__wrapper, img {
      width: 24px !important;
      height: 24px !important;
    }
  }

instead of

  &__button,
 &__icon {
    svg,
 .build-status-icon__wrapper,
 img {
      width: 24px !important;
      height: 24px !important;
    }
  }

or your recommendation

  &__button,
  &__icon {
    svg,
    .build-status-icon__wrapper,
    img {

[basil]

That sounds great!

[basil]

relates to JENKINS-68887

I filed two additional tickets, for a total of three tickets:

• JENKINS-68904 Frontend build is not lint-clean (is blocking JENKINS-68903)
• JENKINS-68903 Frontend lint violations do not fail the build (causes JENKINS-68887, depends on JENKINS-68904)
• JENKINS-68887 CssSyntaxError when linting war module on Windows (caused by JENKINS-68903)

This PR chips away at (but does not fully solve) JENKINS-68904. I opened #6744 to close JENKINS-68887. Once JENKINS-68904 is closed, JENKINS-68903 can be implemented. That will prevent bugs like JENKINS-68887 from ever occurring again.

To track related tickets in one central place, I have created a new epic:

• JENKINS-68905 Frontend build system and dependency technical debt

As time goes on and discovery work is completed to identify frontend build system and dependency related technical debt, additional tickets should be placed in this epic.

(BTW, this is a great example of the power of Jira)

[NotMyFault]

Meh, I don't really like this outcome, but I didn't find a rule that is auto fixable for proper line breaks :/

[basil]

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback. Please see the merge process documentation for more information about the merge process. Thanks!