Added case for non-successful, non-failed builds#140
Conversation
Marking as warning rather than failure.
and that's what happens when you've been working in Groovy for too long. Fixed string literals and lowercase method.
nmuesch
left a comment
nmuesch
left a comment
There was a problem hiding this comment.
Overall looks good, thanks for this! @alanranciato I left a couple minor things. Some of them are unrelated to your direct changes, but would be great to include. Let me know if you'd like me to add those in a separate commit if you don't have the time.
| } else if ("UNSTABLE".equals(builddata.get("result")) || "ABORTED".equals(builddata.get("result")) || "NOT_BUILT".equals(builddata.get("result"))) { | ||
| title.append(" " + builddata.get("result").toString().toLowerCase()); | ||
| payload.put("alert_type", "warning"); | ||
| message = "%%% \n [See results for build #" + number + "](" + buildurl + ") "; |
There was a problem hiding this comment.
Can we move these outside of the conditionals, it looks like the message is the same throughout each of these.
There was a problem hiding this comment.
Done. Changes the message in the title slightly from failed ==> failure and succeeded ==> success. Alert type remains unchanged.
| payload.put("alert_type", "success"); | ||
| payload.put("priority", "low"); | ||
| message = "%%% \n [See results for build #" + number + "](" + buildurl + ") "; | ||
| } else if ("UNSTABLE".equals(builddata.get("result")) || "ABORTED".equals(builddata.get("result")) || "NOT_BUILT".equals(builddata.get("result"))) { |
There was a problem hiding this comment.
Could we possibly setup an enum here for the different result types?
There was a problem hiding this comment.
Utilizing Hudson.model.Result class for conditional.
| payload.put("alert_type", "warning"); | ||
| message = "%%% \n [See results for build #" + number + "](" + buildurl + ") "; | ||
| } else if (builddata.get("result") != null) { | ||
| title.append(" failed"); |
There was a problem hiding this comment.
While we're here, we should probably catch this and only return failed if we are indeed in an error state - https://javadoc.jenkins-ci.org/hudson/model/Result.html.
We could fallback to some unknown instead of failed here in this default case.
There was a problem hiding this comment.
if null, setting to not_built state and warning. Only failure returns error.
String buildResult = builddata.get("result") != null ? builddata.get("result").toString() : Result.NOT_BUILT.toString() ;
1) broke title and message from conditional 2) utilize hudson.model.Result class vs. strings 3) set NULL result to NOT_BUILT vs. failure
Reorder the setting of the result
|
@nmuesch - requested changes checked in and ready for review. |
|
@alanranciato Thanks a lot for this! Looks 💯 I'll go ahead and merge. |
2 participants
Marking as warning rather than failure for build not successful, but not failed.
if ABORTED, UNSTABLE, or NOT_BUILT jenkins spec, set as status = warning and pass the correct build result in the title.
#139