plugin modernization #58

mawinter69 · 2023-05-13T10:12:03Z

The plugin is currently on a very old Jenkins version (1.565). This implies dependencies to other plugins that have been extracted from core since then and prevents me from uninstalling these plugins.

bump to require jenkins 2.361.4
use ionicons
replace some deprecated things with recommended alternatives (e.g. javax.annotation)
fix optOut help for the job property
use newer maven in the test

Pane:

Retry link:

Dark Theme:
Pane:

Retry link:

Testing done

Submitter checklist

Make sure you are opening from a topic/feature/bugfix branch (right side) and not your main branch!
Ensure that the pull request title represents the desired changelog entry
Please describe what you did
Link to relevant issues in GitHub or Jira
Link to relevant pull requests, esp. upstream and downstream changes
Ensure you have provided tests - that demonstrates feature works or fixes the issue

bump to require jenkins 2.361.4 use ionicons replace some deprecated things with recommended alternatives fix optOut help for the job property use newer maven in the test

javax.annotation is deprecated

the default prevents an exception when checking the regexpForMatrixStrategy after adding the naginator publisher for the first time.

mawinter69 · 2023-05-13T10:37:01Z

src/main/java/com/chikli/hudson/plugin/naginator/NaginatorPublisher.java

@@ -229,9 +226,6 @@ public DescriptorImpl getDescriptor() {
        return (DescriptorImpl) super.getDescriptor();
    }

-    @Extension


The Extension annotation is now directly on the NaginatorListener

mawinter69 · 2023-05-13T10:39:46Z

src/main/java/com/chikli/hudson/plugin/naginator/NaginatorPublisher.java

-        /**
-         * Creates a new instance of {@link NaginatorPublisher} from a submitted form.
-         */
-        @Override
-        public Notifier newInstance(StaplerRequest req, JSONObject formData) throws FormException {
-            return req.bindJSON(NaginatorPublisher.class, formData);
-        }
-


The default implementation is doing the same and properly handles the theoretically possible case that req is null

ikedam · 2023-05-14T04:07:52Z

src/test/java/com/chikli/hudson/plugin/naginator/NaginatorCauseTest.java

        FreeStyleProject p = j.createFreeStyleProject();
        FreeStyleBuild build1 = p.scheduleBuild2(0).get();
-        build1.setDisplayName("<div id=\"unescaped-displayname\">bad displayname</div>");
+        build1.setDisplayName("<div id=\"unescaped-displayname\">bad displayname</div>");


Change note: Unprintable whitespace between setDisplayName and ( is removed.

ikedam · 2023-05-14T04:12:54Z

src/main/resources/com/chikli/hudson/plugin/naginator/NaginatorOptOutProperty/help-optOut.html

@@ -1,4 +1,4 @@
 <div>
-    By default, naginator offer a "rebuild" link to all failed builds. Some jobs anyway might not be designed to
+    By default, naginator offers a "retry" link to all failed builds. Some jobs anyway might not be designed to


Change note: retry is correct: https://github.com/jenkinsci/naginator-plugin/blob/naginator-1.18.2/src/main/java/com/chikli/hudson/plugin/naginator/NaginatorRetryAction.java#L51

ikedam · 2023-05-14T04:15:33Z

src/main/java/com/chikli/hudson/plugin/naginator/NaginatorActionFactory.java

@@ -14,13 +14,18 @@
 * @author <a href="mailto:nicolas.deloof@gmail.com">Nicolas De Loof</a>
 */
 @Extension
-public class NaginatorActionFactory extends TransientBuildActionFactory {
+public class NaginatorActionFactory extends TransientActionFactory<AbstractBuild> {


Review note: TransientBuildActionFactory and TransientActionFactory is binary incompatible, but I believe OK
(OK means we don't need to change the major version to mark binary incompatible).
I believe external modules never depend on NaginatorActionFactory.

ikedam · 2023-05-14T04:20:17Z

src/main/resources/com/chikli/hudson/plugin/naginator/NaginatorPublisher/config.jelly

@@ -32,8 +32,7 @@
            </f:entry>
            <j:if test="${descriptor.isMatrixProject(it)}">
                <f:entry title="${%How to apply the regular expression to matrix}" field="regexpForMatrixStrategy">
-                    <!-- unfortunatelly, f:enum doesn't support doCheckXxxx -->
-                    <f:select />
+                    <f:select default="TestParent"/>


context: https://github.com/jenkinsci/naginator-plugin/blob/naginator-1.18.2/src/main/java/com/chikli/hudson/plugin/naginator/NaginatorPublisher.java#L75

Without this when adding the Publisher for the first time an exception is thrown and the UI breaks.

ikedam

Thanks for the request.
The change almost looks good to me. I want to keep title for the icon if we could.

Could you attach screenshots for icons? I think I can try that in the next weekend (unfortunately, I don't have an appropriate environment now), but the screenshot will be helpful if that doesn't bother you.

src/main/resources/com/chikli/hudson/plugin/naginator/NaginatorAction/badge.jelly

ikedam

Thanks for updates and screenshots!

ikedam · 2023-05-19T23:32:54Z

I'm planning makina a new release tomorrow: #60

mawinter69 added 2 commits May 13, 2023 03:10

plugin modernization

e79a8ed

bump to require jenkins 2.361.4 use ionicons replace some deprecated things with recommended alternatives fix optOut help for the job property use newer maven in the test

adjut nonnull and checkfornull annotations

8c65cc4

javax.annotation is deprecated

mawinter69 mentioned this pull request May 13, 2023

use java11 for the build #59

Merged

6 tasks

add default for regexpForMatrixStrategy

ae18bf5

the default prevents an exception when checking the regexpForMatrixStrategy after adding the naginator publisher for the first time.

mawinter69 commented May 13, 2023

View reviewed changes

ikedam reviewed May 14, 2023

View reviewed changes

ikedam requested changes May 14, 2023

View reviewed changes

src/main/resources/com/chikli/hudson/plugin/naginator/NaginatorAction/badge.jelly Outdated Show resolved Hide resolved

add tooltip to badge

c2c88f6

mawinter69 force-pushed the modernize branch from 4676b42 to c2c88f6 Compare May 15, 2023 07:16

ikedam approved these changes May 19, 2023

View reviewed changes

ikedam merged commit 52670b4 into jenkinsci:master May 19, 2023

ikedam mentioned this pull request May 19, 2023

Update CHANGELOG for 1.19 #60

Merged

6 tasks

sghill-rewrite mentioned this pull request Jun 5, 2023

Restore test scope to jenkins-test-harness-tools #64

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

plugin modernization #58

plugin modernization #58

mawinter69 commented May 13, 2023 •

edited

Loading

mawinter69 May 13, 2023

mawinter69 May 13, 2023

ikedam May 14, 2023

ikedam May 14, 2023

ikedam May 14, 2023

ikedam May 14, 2023

mawinter69 May 14, 2023

ikedam left a comment

ikedam left a comment

ikedam commented May 19, 2023

plugin modernization #58

plugin modernization #58

Conversation

mawinter69 commented May 13, 2023 • edited Loading

Testing done

Submitter checklist

mawinter69 May 13, 2023

Choose a reason for hiding this comment

mawinter69 May 13, 2023

Choose a reason for hiding this comment

ikedam May 14, 2023

Choose a reason for hiding this comment

ikedam May 14, 2023

Choose a reason for hiding this comment

ikedam May 14, 2023

Choose a reason for hiding this comment

ikedam May 14, 2023

Choose a reason for hiding this comment

mawinter69 May 14, 2023

Choose a reason for hiding this comment

ikedam left a comment

Choose a reason for hiding this comment

ikedam left a comment

Choose a reason for hiding this comment

ikedam commented May 19, 2023

mawinter69 commented May 13, 2023 •

edited

Loading