Skip to content
This repository has been archived by the owner on Oct 29, 2024. It is now read-only.

Allow disabling comment? #209

Open
joshma opened this issue Mar 21, 2017 · 10 comments
Open

Allow disabling comment? #209

joshma opened this issue Mar 21, 2017 · 10 comments

Comments

@joshma
Copy link

joshma commented Mar 21, 2017

Now that Harbormaster leaves a notice / emails when the build fails, would it be possible to support disabling comments entirely? We currently get a bunch of noise on each failed build since Harbormaster sends an email, and we have 12 different Jenkins builds in parallel that might all error (if e.g. there's a syntax error).

Here's what Harbormaster says:
image

@ascandella
Copy link
Contributor

Have you unchecked the "comment with console link on failure" configuration option?

https://github.com/uber/phabricator-jenkins-plugin/blob/master/docs/configure-job-post-build.png

@joshma
Copy link
Author

joshma commented Mar 22, 2017

Yeah, I did. I believe that only leaves out the link to the console, but it still sends a comment?

https://github.com/uber/phabricator-jenkins-plugin/blob/5a3c257bdc32c852def0341affcb7bcac1230908/src/main/java/com/uber/jenkins/phabricator/BuildResultProcessor.java#L181

@joshma
Copy link
Author

joshma commented Mar 22, 2017

I could try to send a PR, but I haven't written Java in years haha. Seems like you'd have to:

I verified just now that, even without "comment with console link on failure" it still comments something like "Build was aborted https://jenkins.domain/job/project/build/ for more details."

@ascandella
Copy link
Contributor

Aborted is different than failed. Last I checked (and the whole reason I added that checkbox option in the first place) was for this exact scenario -- if you'd like to disable "failed build" comments, check the box. Do you have an example of a build that is sending a comment for a failed (not aborted) build with this option configured properly?

@joshma
Copy link
Author

joshma commented Mar 23, 2017

Hm, well unstable builds comment as well:

Build is unstable https://jenkins.domain/job/build/78251/ for more details.

Maybe the issue then is that it comments for unstable builds? Checking a few failed builds, I indeed don't see the comment.

@ascandella
Copy link
Contributor

Yes, agreed that "unstable" is treated differently than "failed" and that the failed build checkbox option partially solves your problem. The question now is whether we want to add an option to configure "aborted" builds or just treat "aborted" the same as "failed" and let harbormaster send the emails. For simplicity's sake I'd probably lean towards the latter solution (if you have disabled failed build comments, we will also disable aborted build comments).

Thoughts?

@joshma
Copy link
Author

joshma commented Mar 23, 2017

Agreed that you'd probably want aborted to be disabled along with failed (although it'd be nice for the label in the UI to be updated to reflect this).

I'd personally be fine with having one checkbox for all comments - if someone wants a comment on UNSTABLE, it seems like they'd want a comment on FAILED as well. I don't see any use cases where you want one but not the other, but that might just be my limited imagination.

@ascandella
Copy link
Contributor

When you say "the UI to be updated to reflect this", you mean have harbormaster say something other than "failed"? Is that possible with the new harbormaster APIs, e.g. something we could change on our end, or is it still just pass/fail?

@joshma
Copy link
Author

joshma commented Mar 23, 2017

Oops I just mean that, in the Jenkins config UI, the label next to the checkbox should just be updated to indicate whatever behavior is changed.

Right now it's "Comment with console link on Failure", although it might make the most sense to have another option to disable comments entirely.

@ascandella
Copy link
Contributor

ascandella commented Mar 23, 2017 via email

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants