1376829 - Add notes to alerts

Status: RESOLVED FIXED

(Tree Management :: Perfherder, enhancement, P1)

Description

[William Lachance (:wlach)]

It would be useful to be able to add notes to alert summaries -- we could use this to provide information on current sheriffing status, details on the actual revisions, or other details that would make the summary more intuitive.

My current plan is to just store these as plain text as a property of the summary table, then render the information (if available) as markdown on the client.

Comment 1

[Ionuț Goldan [:igoldan]]

:wlach Can I take this bug?

Comment 2

[William Lachance (:wlach)]

(In reply to Ionuț Goldan [:igoldan], Performance Sheriffing from comment #1)
> :wlach Can I take this bug?

For sure, I started work on this last year in San Francisco's hackfest but I only got maybe 10-15% finished before I realized this would be a lot more work than I thought. I don't see myself returning to this in the forseable future. In case it's useful, I uploaded the branch I was working on here:

https://github.com/wlach/treeherder/commits/perfherder-notes

Comment 3

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3345

Comment 4

[Ionuț Goldan [:igoldan]]

Comment on attachment 8959078 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3345

Review of attachment 8959078 [details] [review]:
-----------------------------------------------------------------

I added the notes functionality. Please test it a little and tell whether something extra is required.

Comment 5

[Joel Maher ( :jmaher ) (UTC -8)]

Comment on attachment 8959078 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3345

in general this looks pretty straightforward.  I didn't run it locally to see how the look and feel is, but from a code perspective I added a single comment about a variable name.

Comment 6

[Ed Morley [:emorley]]

Comment on attachment 8959078 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3345

(Restoring attachment metadata; I'd avoid using the "edit attachment as" / splinter review functionality since it breaks these fake link attachments. Use the "details" link instead to add `review?x`)

Comment 7

[William Lachance (:wlach)]

Comment on attachment 8959078 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3345

Left some suggestions in the PR, in general this looks good.

Comment 8

[William Lachance (:wlach)]

Comment on attachment 8959078 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3345

Very close, just a few more things I'd like to see changed.

Comment 9

[Ionuț Goldan [:igoldan]]

Comment on attachment 8959078 [details] [review]
Link to GitHub pull-request: https://github.com/mozilla/treeherder/pull/3345

I removed the drawer and instead added the Add/Edit commands into the dropdown.
The notes preview is under the individual alerts.

Comment 10

[William Lachance (:wlach)]

Just a note: Ionut's implementation doesn't render any markdown in the note. This was probably the right trade off initially, but might be something we want to add in the future.

Comment 11

[Treeherder GitHub Bugbot]

Commit pushed to master at https://github.com/mozilla/treeherder

https://github.com/mozilla/treeherder/commit/aa40c6a7e490a7907d7afe87e35ce49ea4e0d842
Bug 1376829 - Add notes to alert summaries (#3345)

Comment 12

[Joel Maher ( :jmaher ) (UTC -8)]

on production