-
-
Notifications
You must be signed in to change notification settings - Fork 8.6k
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
[JENKINS-68801] - Fix for Functions#extractPluginNameFromIconSrc #6902
Conversation
Hi @janfaracik @timja |
/reviewer @jenkinsci/sig-ux |
Is this a theoretical issue or do you have a plugin which includes the text |
We have plugin which contains "plugin-" in the artifactId |
(untested, but I assume you have) |
/reviewer @jenkinsci/sig-ux |
/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! |
I have tested it and it works fine |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks reasonable to me.
Do we maybe want to add a tiny unit test for this? E.g. FunctionsTest.java#L398
Added test for new functionality |
I went ahead and fixed your spot bugs issues introduced, hope you don't mind. /label ready-for-merge This PR is now ready for merge. We will merge it after ~24 hours if there is no negative feedback. |
I wasn't able to add the following labels: ready-for-merge This PR is now ready for merge Check that the label exists and is spelt right then try again. |
(labelling with other text is currently broken, it will work on it's own) |
…kinsci#6902) Co-authored-by: Temirlan Dyussyumbayev <bzzitsme@users.noreply.github.com> Co-authored-by: Alexander Brandes <mc.cache@web.de> (cherry picked from commit 83bf697)
See JENKINS-68801.
If our plugin's artifactId will contain "plugin-", then it will replace all "plugin-" occurences instead of first one.
Then it won't be able to correctly extract plugin name
Proposed changelog
Proposed upgrade guidelines
N/A
Submitter checklist
Proposed changelog entries
section only if there are breaking changes or other changes which may require extra steps from users during the upgrade@Restricted
or have@since TODO
Javadoc, as appropriate.@Deprecated(since = "TODO")
or@Deprecated(forRemoval = true, since = "TODO")
if applicable.Desired reviewers
@timja @janfaracik
Maintainer checklist
Before the changes are marked as
ready-for-merge
:Proposed changelog entries
are accurate, human-readable, and in the imperative moodupgrade-guide-needed
label is set and there is aProposed upgrade guidelines
section in the PR title. (example)lts-candidate
to be considered (see query).