Remove not anymore required BuildTimeOutJenkinsRule

jenkinsci / build-timeout-plugin

Jenkins build-timeout plugin

https://plugins.jenkins.io/build-timeout/

31 stars 80 forks source link

Remove not anymore required BuildTimeOutJenkinsRule #72

Closed darxriggs closed 2 years ago

darxriggs commented 4 years ago

The cause why BuildTimeOutJenkinsRule was created initially seems to be (based on the code comment "not public till Jenkins 1.479") that one or more methods haven't been public and therefore haven't been available for testing. As currently Jenkins 1.625.1 is the minium required version they are public now and JenkinsRule can be used directly.

darxriggs commented 4 years ago

@ikedam I updated the description. Please have a look again.

ikedam commented 4 years ago

@darxriggs Sorry for my slow response.

Actually, I don’t plan to apply new changes to build-timeout plugin as long as there’s no actual problem as build-timeout is fundamentally incompatible with pipelines and no new features won’t be introduced. I believe improving tests make nothing as no more development is planned.

So what I expect in the description is: why you want to improve test codes. For example, you plan to add new features to build-timeout (I’m really happy if it was to make build-timeout compatible with pipeline), the build-timeout causes problems in some cases and it cannot be exposes with current tests, or you plan to update tests and target versions of all Jenkins plugin (maybe recommended plugins). I no longer maintain this plugin hard, actually, I’m not the official maintainer of this plugin, and it may make sense to have you adopt this plugin if you plan to make a series of changes to this plugin.

darxriggs commented 4 years ago

I am currently not planning to add features to this plugin. My goal is to clean up some pieces (in this case remove unnecessary code) with minimal effort. I already did so successfully for many other plugins over the past year.

I think cleaning up the existing tests is a good idea even if there are no big future plans for this plugin. Therefore I ask you to merge this improvement.

ikedam commented 4 years ago

I don’t review or merge this request (at least for now) for following reasons:

This change does nothing for users of plugins.
I rather prefer preserving existing codes, as long as the existing code actually doesn’t cause problems.
I don’t think those activities are necessary for this plugin that isn’t actively maintained.

It doesn’t mean rejecting this request, and it can be reviewed and merged when more features are introduced in future.

darxriggs commented 4 years ago

As there has been recent activity for this plugin, I rebased this pull request on master.

@ikedam Therefore I would like you to reconsider it.