Remove dead JS code #9136

zbynek · 2024-04-06T21:13:50Z

The YUI.log was most likely a typo (was meant to be YAHOO.log) and is not needed anyway since the standard console.log is used in other places. Checks for existence of console can be dropped since they are missing in other places. Some global vars are not needed in anymore (mostly because of Prototype removal).

No tests were added/changed because functionality didn't change.
geval code was simplified by removing IE-only hacks, but it remains incompatible with CSP (on purpose)

Testing done

JS lint passes

Proposed changelog entries

(skip changelog)

Proposed upgrade guidelines

N/A

Submitter checklist

Give feedback

The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see [examples](https://github.com/jenkins-infra/jenkins.io/blob/master/content/_data/changelogs/weekly.yml)). Fill in the **Proposed upgrade guidelines** section only if there are breaking changes or changes that may require extra steps from users during upgrade.

The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
Options
Successfully updated the issue's project

There was an error updating the issue's project
There is automated testing or an explanation as to why this change has no tests.

There is automated testing or an explanation as to why this change has no tests.
Options
Successfully updated the issue's project

There was an error updating the issue's project
New public classes, fields, and methods are annotated with `@Restricted` or have `@since TODO` Javadocs, as appropriate.

New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
Options
Successfully updated the issue's project

There was an error updating the issue's project
New deprecations are annotated with `@Deprecated(since = "TODO")` or `@Deprecated(forRemoval = true, since = "TODO")`, if applicable.

New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
Options
Successfully updated the issue's project

There was an error updating the issue's project
For dependency updates, there are links to external changelogs and, if possible, full differentials.

For dependency updates, there are links to external changelogs and, if possible, full differentials.
Options
Successfully updated the issue's project

There was an error updating the issue's project
For new APIs and extension points, there is a link to at least one consumer.

For new APIs and extension points, there is a link to at least one consumer.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Options

Desired reviewers

@mention

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

Maintainer checklist

Give feedback

There are at least two (2) approvals for the pull request and no outstanding requests for change.

There are at least two (2) approvals for the pull request and no outstanding requests for change.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.

Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Changelog entries in the pull request title and/or **Proposed changelog entries** are accurate, human-readable, and in the imperative mood.

Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
Options
Successfully updated the issue's project

There was an error updating the issue's project
Proper changelog labels are set so that the changelog can be generated automatically.

Proper changelog labels are set so that the changelog can be generated automatically.
Options
Successfully updated the issue's project

There was an error updating the issue's project
If the change needs additional upgrade steps from users, the `upgrade-guide-needed` label is set and there is a **Proposed upgrade guidelines** section in the pull request title (see [example](https://github.com/jenkinsci/jenkins/pull/4387)).

If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
Options
Successfully updated the issue's project

There was an error updating the issue's project
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](https://issues.jenkins.io/issues/?filter=12146)).

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).
Options
Successfully updated the issue's project

There was an error updating the issue's project
Options

timja · 2024-04-07T07:12:36Z

/label ready-for-merge

This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

MarkEWaite · 2024-05-14T11:55:55Z

This seems to have caused a regression when creating a new certificate credential with PKCS#12 type. @zbynek can you investigate JENKINS-73166 to confirm that my git bisect is correct and to identify a fix?

MarkEWaite · 2024-05-14T12:16:12Z

As far as I can tell, I had to restore the following subset of the commit:

 function geval(script) {
-  eval(script);
+  // execScript chokes on "" but eval doesn't, so we need to reject it first.
+  if (script == null || script == "") {
+    return;
+  }
+  // see http://perfectionkills.com/global-eval-what-are-the-options/
+  // note that execScript cannot return value
+  (this.execScript || eval)(script);
 }

When I replace the (this.execScript || eval)(script); with eval(script) it fails.

The replacement of the YUI logging lines with console.log did not seem to affect JENKINS-73166.

https://issues.jenkins.io/browse/JENKINS-73166 notes that the credentials code for certificates has a dependency on this JavaScript code. Without this JavaScript, an attempt to add a new certificate credential type using PKCS#12 will fail with an HTTP 404 error. The replacement of YUI.log with console.log is not involved in the bug report, so it is retained in the code change. This reverts part of commit 932ab19c5448bb0afa46f60b93c245eb08e86b55.

Remove dead JS code

b416a59

zbynek force-pushed the remove-dead-js branch from c7caba1 to b416a59 Compare April 6, 2024 23:11

timja approved these changes Apr 7, 2024

View reviewed changes

timja added the skip-changelog Should not be shown in the changelog label Apr 7, 2024

comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Apr 7, 2024

NotMyFault approved these changes Apr 7, 2024

View reviewed changes

NotMyFault merged commit 89c7754 into jenkinsci:master Apr 8, 2024
17 checks passed

MarkEWaite mentioned this pull request May 17, 2024

[JENKINS-73166] Fix PKCS#12 certificate save to credentials #9284

Closed

zbynek mentioned this pull request May 17, 2024

[JENKINS-73166] Fix PKCS#12 certificate save to credentials #9286

Merged

zbynek deleted the remove-dead-js branch May 17, 2024 19:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove dead JS code #9136

Remove dead JS code #9136

zbynek commented Apr 6, 2024 •

edited

Loading

Submitter checklist

Maintainer checklist

timja commented Apr 7, 2024

MarkEWaite commented May 14, 2024

MarkEWaite commented May 14, 2024 •

edited

Loading

Remove dead JS code #9136

Remove dead JS code #9136

Conversation

zbynek commented Apr 6, 2024 • edited Loading

Testing done

Proposed changelog entries

Proposed upgrade guidelines

Submitter checklist

Desired reviewers

Maintainer checklist

timja commented Apr 7, 2024

MarkEWaite commented May 14, 2024

MarkEWaite commented May 14, 2024 • edited Loading

zbynek commented Apr 6, 2024 •

edited

Loading

MarkEWaite commented May 14, 2024 •

edited

Loading