Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace hetero-list YUI button and menu with new style button and tippy.js menu #8418

Merged
merged 20 commits into from
Aug 30, 2023

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented Aug 22, 2023

same as #8340 which was reverted with #8412

Testing done

Proposed changelog entries

  • Update several buttons and menus to replace YahooUI in more locations.

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@mention

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

Maintainer checklist

@github-actions github-actions bot added the unresolved-merge-conflict There is a merge conflict with the target branch. label Aug 23, 2023
@github-actions
Copy link
Contributor

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

@github-actions github-actions bot removed the unresolved-merge-conflict There is a merge conflict with the target branch. label Aug 23, 2023
daniel-beck added a commit to daniel-beck/jenkins that referenced this pull request Aug 24, 2023
@NotMyFault NotMyFault added web-ui The PR includes WebUI changes which may need special expertise rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted security-approved @jenkinsci/core-security-review reviewed this PR for security issues labels Aug 25, 2023
Copy link
Member

@basil basil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!

@basil basil added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Aug 29, 2023
@basil basil merged commit 0c71e41 into jenkinsci:master Aug 30, 2023
16 checks passed
car-roll pushed a commit to jenkinsci/script-security-plugin that referenced this pull request Aug 31, 2023
* forward compatibility with core-8418

jenkinsci/jenkins#8418

* revert pom change

* check via versionnumber
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe you are already aware of this issue, but other plugins used the adjunct directly and the deletion of this file at least causes warnings (IDK if it breaks things or not). See this search. You can reproduce the warnings by opening a page that uses the adjunct (e.g. a Multibranch Pipeline configuration page) while running the latest weekly. Here is what the warnings look like:

2023-09-26 16:19:11.029+0000 [id=116]	WARNING	o.k.s.f.adjunct.AdjunctsInPage#findNeeded: No such adjunct found: lib.form.hetero-list.hetero-list
org.kohsuke.stapler.framework.adjunct.NoSuchAdjunctException: Neither lib.form.hetero-list.hetero-list.css, .js, .html, nor .jelly were found
	at org.kohsuke.stapler.framework.adjunct.Adjunct.<init>(Adjunct.java:125)
	at org.kohsuke.stapler.framework.adjunct.AdjunctManager.get(AdjunctManager.java:148)
	at org.kohsuke.stapler.framework.adjunct.AdjunctsInPage.findNeeded(AdjunctsInPage.java:161)
	at org.kohsuke.stapler.framework.adjunct.AdjunctsInPage.generate(AdjunctsInPage.java:113)
	at org.kohsuke.stapler.jelly.AdjunctTag.doTag(AdjunctTag.java:82)
        ... more Jelly/Jetty/Stapler stack frames ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The adjunct declaration can be removed in the plugins once they require Jenkins 2.422 or newer.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The adjunct declaration can be removed in the plugins once they require Jenkins 2.422 or newer.

and unless that happens your logs now contain massive stack traces :(

@MarkEWaite
Copy link
Contributor

Confirmed by a user that the multibranch Pipeline plugin no longer allows the addition of multiple branch sources after the first save. Described in JENKINS-72170 and confirmed by bisect that this commit introduced the issue.

I think that a fix for this will need to be included as a backport into Jenkins 2.426.1 LTS. I'm trying some alternatives now.

@mawinter69
Copy link
Contributor Author

taking a look already

@MarkEWaite

This comment was marked as outdated.

@mawinter69
Copy link
Contributor Author

Fixed with #8602

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted security-approved @jenkinsci/core-security-review reviewed this PR for security issues web-ui The PR includes WebUI changes which may need special expertise
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants