[JENKINS-43930 & Others] - FindBugs fixes towards 1.9 #14
Conversation
…rect.log is not writable
|
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
| task.getLogger().println(r == 0 ? "Successfully started" : "Start service failed. Exit code=" + r); | ||
| } catch (IOException | InterruptedException ex) { | ||
| // Level is severe, because the process won't be recovered in the service mode | ||
| LOGGER.log(Level.SEVERE, "Failed to start the service with id=" + serviceId, ex); |
There was a problem hiding this comment.
The only change is logging here
| <plugin> | ||
| <groupId>org.codehaus.mojo</groupId> | ||
| <artifactId>findbugs-maven-plugin</artifactId> | ||
| <configuration> |
There was a problem hiding this comment.
The default threshold in plugin-pom is Medium, hence it misses the File leak on exceptional path in this case. Ideally should be fixed in Parent POM, of course
There was a problem hiding this comment.
File leaks are treated as low severity by FindBugs? Maybe that is what should be fixed.
There was a problem hiding this comment.
- File leaks on the common path are the Medium/High Severity depending on the detector
- File leaks on Exceptional paths are low
The latter bullet does not make me happy as well
| @@ -26,7 +27,9 @@ public SlaveInstaller createIfApplicable(Channel c) throws IOException, Interrup | |||
| return null; | |||
| } | |||
|
|
|||
| @SuppressFBWarnings(value = "SE_NO_SERIALVERSIONID", justification = "Remoting does not need it") | |||
There was a problem hiding this comment.
Maybe it is safe in this case, not sure
|
@reviewbybees done |
@reviewbybees