871588 - Add more tree-closure reason tags to Tree Status, to aid stats generation

Status: RESOLVED FIXED

(Release Engineering :: General, defect)

Description

[Ed Morley [:emorley]]

Currently treestatus has an "infra" checkbox, set when closing the tree, to indicate that the cause was due to infrastructure problems. This field is optional, and as such often forgotten. In addition, we could really do with more tags, so that we can generate stats of tree-closure reasons.

Suggested tags:
* bustage (maybe split into 'compilation-failure' and 'test-failure' ?)
* infra (maybe split further, eg: infra-ftp, infra-colo)
* capacity (or 'backlog')
* other

...any more? (eg 'large-landing', 'planned-closure', 'not-in-use', ...)

Comment 1

[Ed Morley [:emorley]]

Attachment: Patch v1

1) Adds the following tags (in addition to the existing 'infra' tag):
* checkin-compilation
* checkin-test
* backlog
* planned
* other
2) Makes specifying at least one tag (and one tree, whilst we're at it) mandatory via the UI.
3) Validates that we've set tags server side (given our API) & that they're non-empty (we would have previously stored empty string tags).

Note the duplication in tree.html is horrid, but I will make it go away shortly, once I leave it a bit longer to see if https://github.com/mozilla/treestatus/pull/29 has a response from the its original owner. I'll also tweak the CSS a bit, so the layout is nicer and we don't keep on using break tags all over the place.

Comment 2

[Ed Morley [:emorley]]

catlee, don't suppose you know when you might be able to look at this? :-)

Comment 3

[Ed Morley [:emorley]]

(In reply to Ed Morley [:edmorley UTC+1] from comment #2)
> catlee, don't suppose you know when you might be able to look at this? :-)

Comment 4

[Chris AtLee [:catlee]]

Comment on attachment 749323 [details] [diff] [review]
Patch v1

Review of attachment 749323 [details] [diff] [review]:
-----------------------------------------------------------------

::: treestatus/templates/index.html
@@ +114,5 @@
> +                  <input type="checkbox" name="tags" value="backlog" id="backlog" onclick="validateForm()" />
> +                  <label for="backlog">Job backlog?</label><br/>
> +
> +                  <input type="checkbox" name="tags" value="planned" id="planned" onclick="validateForm()" />
> +                  <label for="planned">Planned closure?</label><br/>

I'd prefer to drop the '?' from the labels here. Is there a reason you feel we need them?

::: treestatus/templates/tree.html
@@ +79,5 @@
> +                    <input type="checkbox" name="tags" value="planned" id="planned" onclick="validateForm()" />
> +                    <label for="planned">Planned closure?</label><br/>
> +
> +                    <input type="checkbox" name="tags" value="other" id="other" onclick="validateForm()" />
> +                    <label for="other">Other</label><br/><br/>

Can we factor out the list of tags and labels somewhere so it's not duplicated here and in index.html?

Comment 5

[Chris AtLee [:catlee]]

Comment on attachment 749323 [details] [diff] [review]
Patch v1

just noticed that you mentioned factoring out the common bits of html in the github pull request. hope that can happen soon!

Comment 6

[Ed Morley [:emorley]]

(In reply to Chris AtLee [:catlee] from comment #4)
> I'd prefer to drop the '?' from the labels here. Is there a reason you feel
> we need them?

Done :-)

(In reply to Chris AtLee [:catlee] from comment #5)
> just noticed that you mentioned factoring out the common bits of html in the
> github pull request. hope that can happen soon!

Yup will look at that in the next week :-)

Landed with nits fixed & one final test locally:
https://github.com/mozilla/treestatus/commit/df1863a6324e0b168b034f339e53d8b2141e6452

Comment 7

[Ed Morley [:emorley]]

In production :-)