unexpected npe fixes for findbugs #70
Conversation
|
Thank you for a pull request! Please check this document for how the Jenkins project handles pull requests |
| @@ -55,8 +55,8 @@ protected EnvVars buildEnv(AbstractProject<?, ?> job) { | |||
| for (EnvironmentContributor contributor : EnvironmentContributor.all()) { | |||
| try { | |||
| contributor.buildEnvironmentFor(job, env, TaskListener.NULL); | |||
| } catch (Exception e) { | |||
| LOGGER.debug("{} failed to build environment ({})", contributor.getClass(), e.getMessage()); | |||
| } catch (Throwable t) { | |||
There was a problem hiding this comment.
After the discussions in other PRs we agreed that this approach is not good, because it suppresses various kinds of system Errors, which are supposed to be system failures
There was a problem hiding this comment.
so catch only Exception ?
|
I'm mostly OK with the fix, but actually it makes the further merge even harder. |
|
@oleg-nenashev All codebase has changed. This PR fix 99% of current fb issues so your pr can be just skipped as i have plan to connect sonarQube |
|
Yes, seems so. BTW it makes sense to apply the POM changes. I'll do it latet |
| } | ||
|
|
||
| @Override | ||
| public int hashCode() { |
There was a problem hiding this comment.
wouldn't this influence on queue?
|
We (with @KostyaSha) decided not to change equals and hashcode in cause object as of it may brake something in job sheduling |
unexpected npe fixes for findbugs
contains rewrited parts from #63 (but not whole)
plz review @oleg-nenashev
think other findbugs issues can be ignored