jenkinsci / build-timeout-plugin

Jenkins build-timeout plugin
https://plugins.jenkins.io/build-timeout/
31 stars 80 forks source link

Modernize plugin #64

Closed daniel-beck closed 6 years ago

daniel-beck commented 6 years ago

Tests still pass, otherwise I'm not confident I didn't mess up some of the internals. Review is needed.

daniel-beck commented 6 years ago

If you're concerned about the core dependency increase: http://stats.jenkins.io/pluginversions/build-timeout.html 1.18 is 10 months old (plenty of time to update) and 100% of its users are on that core version or newer (96% of all users of all versions of the plugin are).

TBH if it were my plugin, I'd go with a 2.x release, but the main point here is to get rid of the unused implied dependencies to detached plugins.

ikedam commented 6 years ago

Upgrading the core looks reasonable to me. Targetting old cores cause installing unnecessary plugins as you point, and build-timeout is often installed during the setup wizard. (At the same time, build-timeout doesn't support pipeline, and may not be useful for many people)

I wonder why ci.jenkins.io doesn't test this request even after merging #65 ...

ikedam commented 6 years ago

Test failures caused for

So you can fix the test failures by modifying the log line from FakeBuildStep to more appropriate identifiable string.

And we can replace BuildStepWithTimeoutTest#assertLogDoesNotContain to JenkinsRule#assertLogNotContains, which makes test errors more descriptive.

daniel-beck commented 6 years ago

@ikedam I addressed your review comments.

ikedam commented 6 years ago

Released build-timeout-1.19. It allows users install the plugin without installing matrix-project so on.

daniel-beck commented 6 years ago

@ikedam Thanks!