Address eslint across all JS files

[timja]

Previously it was just looking at the war module

See #6863 (comment)

Testing done

• I've tested the components
• I've clicked through pages looking for JavaScript errors
• I've checked expected global objects are still present.

This should be safe although 95% of changes were all manual, --fix wasn't much help here.

I manually cleaned up some styling issues introduced by --fix, consistent styling will be applied by #6863

Proposed changelog entries

• (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(forRemoval = true, since = "TODO") if applicable.
• New or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content-Security-Policy directives (see documentation on jenkins.io).
• 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).

[NotMyFault]

If we add it to the root directory, can we get a rid of the config in the war directory?

[timja]

If we add it to the root directory, can we get a rid of the config in the war directory?

Thanks, I meant to do that, fixed in ee970ee

[daniel-beck]

All code changed in this PR uses two space indentation, despite files currently (mostly?) being indented with four spaces. This introduces mixed indentation which makes no sense.

[daniel-beck]

Unfortunate?

[timja]

I manually fixed all the ones I found, it's what eslint did with the --fix. I will fix this

[timja]

All code changed in this PR uses two space indentation, despite files currently (mostly?) being indented with four spaces. This introduces mixed indentation which makes no sense.

Can we wait for the prettier PR to address that, I said this in the description:

I manually cleaned up some styling issues introduced by --fix, consistent styling will be applied by #6863

(which may not have been clear but yes every edit was somewhat difficult to stay consistent with the inconsistent formatting, this was asked for as a pre-cursor to #6863 to fix some issues with formatting by applying braces consistently.)

[basil]

Very nice! I think this will systematically address Wadeck's concern from the other PR. 👍

I did not review each line for correctness, since I am not a JavaScript programmer. I simply trust that you did the lint fixes correctly.

I agree that the indentation of this PR is not all that important if we plan to auto-format the files afterward anyway. The main goal of this PR, in my mind, is to prepare for the auto-formatting step by structuring the input such that the auto-formatting step can create the best possible output.

[daniel-beck]

consistent styling will be applied by #6863

Thanks, I didn't make the connection that this refers to the inconsistent indentation introduced here.

However,

this was asked for as a pre-cursor to #6863 to fix some issues with formatting

agree that the indentation of this PR is not all that important if we plan to auto-format the files afterward anyway

This looks a lot like both changes should be a single PR, since neither can stand alone without the other? AFAIUI, this one needs the other for indentation to not be a giant inconsistent mess, and the other needs this one to produce tolerable results?

(Also, I would expect that both may create conflicts in open PRs, at least ones involving existing JS files, so I'd rather not have the bot request merge conflict resolution while the other PR is still pending.)

[timja]

This looks a lot like both changes should be a single PR, since neither can stand alone without the other? AFAIUI, this one needs the other for indentation to not be a giant inconsistent mess, and the other needs this one to produce tolerable results?

If that's the preferred option then I can include in #6863

Pushed as https://github.com/jenkinsci/jenkins/pull/6863/commits

(there was an incompatibility in this PR that affected the prettier one, moving the eslint config file to the root prevented loading a package from the war directory so I reverted moving it and passed the config file explicitly so it didn't try look up another one)

---

I've kept this PR open in case it's preferred to merge this separately

[basil]

If we are going to squash #6863 (as we usually do) then I would prefer for this PR to be (squash) merged separately so that the final state is two distinct commits, as I feel this change is different enough from #6863 (automatically applied vs manual changes) that I would want to be able to view them separately when reading the history.

But if we plan to merge #6863 as multiple commits (which we may very well want to do in order to make it easier to cherry-pick the non-automatic portions onto backport branches), then this concern does not apply.

[timja]

I do not plan to squash merge the prettier PR, it has 3 very distinct commits.

personally I would prefer to merge this separately, it’s trivial to fix prettier conflicts as you can apply the automated changes, but if conflicts appear with the prettier changes and this PR it will be more difficult.

although I’m not expecting many conflicts from this PR

Changes addressed

[github-actions]

Please take a moment and address the merge conflicts of your pull request. Thanks!

[basil]

Closing in favor of #6863.