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

Fix for #364 #365

Closed
wants to merge 3 commits into from
Closed

Fix for #364 #365

wants to merge 3 commits into from

Conversation

mawatech
Copy link
Contributor

This PR should fix issue #364

Used jenkinsci/groovy-postbuild-plugin@4aee649 as reference how to fix it.

Short test on a local Jenkins instance showed no problems.

@dcendents
Copy link
Contributor

@mawatech if I'm not mistaken the BadgeAction has been moved to a new plugin, so technically there is no need to have a dependency on the groovy-postbuild plugin

I suggest you change the dependency in the pom

        <dependency>
            <groupId>org.jenkins-ci.plugins</groupId>
            <artifactId>badge</artifactId>
            <version>1.2</version>
        </dependency>

@@ -166,7 +166,7 @@
<dependency>
<groupId>org.jvnet.hudson.plugins</groupId>
<artifactId>groovy-postbuild</artifactId>
<version>2.3.1</version>
<version>2.4</version>
<optional>true</optional>
</dependency>
Copy link
Member

Choose a reason for hiding this comment

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

+1 for the suggestion from @dcendents .
This should be replaced with:

        <dependency>
            <groupId>org.jenkins-ci.plugins</groupId>
            <artifactId>badge</artifactId>
            <version>1.2</version>
            <optional>true</optional>
        </dependency>

(adding optional=true to the dcendents's comment)

And the plugin name to test should be also replaced to badge:
https://github.com/jan-molak/jenkins-build-monitor-plugin/blob/v1.11%2Bbuild.201701152243/build-monitor-plugin/src/main/java/com/smartcodeltd/jenkinsci/plugins/buildmonitor/viewmodel/JobViews.java#L18

Copy link
Member

@ikedam ikedam left a comment

Choose a reason for hiding this comment

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

Hi, I'm a maintainer of groovy-postbuild and I caused this problem. Really sorry.
I think this is the best way to fix the problem.
Build-monitor-plugin should depend directly on badge-plugin rather than groovy-postbuild, please update the dependency definition in pom.xml.

@ikedam
Copy link
Member

ikedam commented Mar 31, 2018

Badge plugin (and groovy-postbuild-2.4+) targets Jenkins 2.60.3 and acceptance test may not work correct. (I don't have an appropriate environment and haven't tried it...)
It may requires also upgrading the target version of build-monitor to 2.60.3.

@jan-molak
Copy link
Member

@mawatech, @ikedam - thanks, I'll have a look at this PR now

@jan-molak
Copy link
Member

jan-molak commented Mar 31, 2018

Looks like the badge-related acceptance tests (mvn clean verify) are failing for this PR:

scaled_2116ea922ddf9e7843f32338800b578b

Does this mean that we need to update the Jenkins core version Build Monitor relies on? @mawatech, @ikedam, @dcendents?

@jan-molak
Copy link
Member

OK, so upgrading to Jenkins 2.x looks like a fairly large piece of work. I'll have a look into that, but can't really promise any timeframes. Any help would be appreciated.

@ikedam
Copy link
Member

ikedam commented Apr 22, 2018

I’m afraid upgrading the core version takes some more time.

@jan-molak I’m thinking of releasing groovy-postbuild-2.4.1 in the following way. Would you comment for this idea?

  • Introduce GroovyPostBuildAction as a subclass of BadgeAction of badge-plugin.
  • It resolves the startup failure.
  • It results HasBadges not work at all, as actual actions are no longer GroovyPostBuildAction but BadgeAction (allDetailsOf() always return an empty list).
  • I believe this is not the best way at all, but is better for now.

@jan-molak
Copy link
Member

@ikedam - I think you're right. I spent a couple of hours looking into upgrading the jenkins core dependency, but it's not a straightforward process as the maven verifier they now have in place requires upgrading a ton of other dependencies.

I'm not that familiar with the internals of the groovy-postbuild plugin, I'm afraid, but if you'd like to try out to implement your suggestion I'll do my best to help out.

Jan

@erikhakansson
Copy link
Contributor

I've merged this into my #370 and also implemented backwards compatibility. This PR alone will break compatibility with old Jenkins' using the old Groovy Postbuild plugin.

@jan-molak
Copy link
Member

Closed by #370

@jan-molak jan-molak closed this May 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants