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

Many Improvements #24

Merged
merged 16 commits into from
Oct 10, 2019
Merged

Many Improvements #24

merged 16 commits into from
Oct 10, 2019

Conversation

darxriggs
Copy link
Contributor

@darxriggs darxriggs commented Oct 5, 2019

@ndeloof can you have a look at this or ask a colleague to do so?

The remove() operation performs this check itself.
Running FindBugs is already configured in the parent pom.
Due to a different execution id FindBugs was run twice.
Using Java 8 as Java 7 is end of life since quite a while.
The removed plugins are already defined in the parent pom.
It's annotated with `@Nonnull` now and throws an exception if no instance
is available. Therefore `null` checks are not required anymore.
`getRootPath()` is annotated with `@Nonnull`, therefore never returns `null`.
@ndeloof
Copy link
Contributor

ndeloof commented Oct 5, 2019

I left Cloudbees 2 months ago ;)

@darxriggs
Copy link
Contributor Author

darxriggs commented Oct 5, 2019

@ndeloof good luck.

@aheritier who could have a look at this?

@darxriggs darxriggs mentioned this pull request Oct 5, 2019
Copy link
Member

@aheritier aheritier left a comment

Choose a reason for hiding this comment

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

It's good for me. It's a good cleanup to update the plugin.

@@ -37,14 +37,9 @@
*/
@Initializer(after = InitMilestone.JOB_LOADED)
public static void initialize() {
// TODO switch to Jenkins.getActiveInstance() once 1.590+ is the baseline
Jenkins jenkins = Jenkins.getInstance();
Copy link
Member

Choose a reason for hiding this comment

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

I think it's ok to be compatible with 2.60+ (it's changing too often :-) )

@aheritier aheritier merged commit c83a9e4 into jenkinsci:master Oct 10, 2019
@darxriggs darxriggs deleted the code-cleanup branch October 10, 2019 20:46
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.

3 participants