-
Notifications
You must be signed in to change notification settings - Fork 12
buildkitereporter: add owner to annotations #76
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
Conversation
|
|
||
| for _, task := range failedTasks { | ||
| if task.Owner != "" { | ||
| fmt.Fprintf(&buf, "Owner: %s\n", task.Owner) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is great. It would be so nice if we could include a link to the Slack ask channel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed - unsure if we want to keep adding attributes to these tasks with say, a link to reach the owner, or if we want to encourage teams to include their communication details in the owner string? hard to say since I think it's a bit unknown how others pulling in Taskrunner use it.
dansamsara
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
ran-arigur
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense — one comment below.
| if task.Owner != "" { | ||
| fmt.Fprintf(&buf, "Owner: %s\n", task.Owner) | ||
| } | ||
| fmt.Fprintf(&buf, "<details><summary><code>%s</code></summary>\n<pre>", task.Name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor: In the interest of hopefully making it a bit clearer what the "owner" actually owns, I'd suggest something like:
ownerMsg := ""
if task.Owner != "" {
ownerMsg = " (task owner: " + task.Owner + ")"
}
fmt.Fprintf(&buf, "<details><summary><code>%s</code>%s</summary>\n<pre>", task.Name, ownerMsg)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Task owner makes sense! I'll update. Do you prefer the owner message below the code summary then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had thought to put it inside the summary, but come to think of it, maybe it does make more sense to put it below the summary, so that you see it when you expand? (Not sure.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I like it out of the summary just so it's easy to discern amongst the actual log output - but below makes sense, you should be able to see it both when the summary is expanded and not expanded. Will update!
2d93fc2 to
0c4f703
Compare
Pull Request Test Coverage Report for Build 15310622079Details
💛 - Coveralls |
0c4f703 to
ba1bf47
Compare
Now that we have added optional owners to Taskrunner tasks, in the event a task fails in a Buildkite workflow, this will include the owner in the resulting annotation.
ba1bf47 to
a761df5
Compare
Now that we have added optional owners to Taskrunner tasks, in the event a task fails in a Buildkite workflow, this will include the owner in the resulting annotation.
Example:

Above task has an owner, task below does not
Update to put owner below summary:
![Uploading Screenshot 2025-05-28 at 2.03.51 PM.png…]()