1263078 - Don't allow "empty" intermittent classifications

Status: RESOLVED FIXED

(Tree Management :: Treeherder, defect)

Description

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

At the moment, all you have to do to classify a failure is to pin a job and click "Save".

I propose that we might change it so a little more effort is required, such that the Save button does not allow clicks until at least one job is pinned AND at least one of these conditions is met:
1) a bug has been added as a related bug
2) the classification type is NOT "intermittent"
3) the classification type IS "intermittent" AND a classification comment has been entered

This should help make sure every failure has at least some thought put into it. You can still mass-classify things by either changing the classification type to something else (like "infra" if something in infra is broken) or you add something (anything!) to the classification comment.


Thoughts?

I have a WIP branch I'm about to push as a starting point.

Comment 1

[GitHub Autolander Bot]

Attachment: [treeherder] KWierso:disablesubmitwithoutanything > mozilla:master

Comment 2

[Carsten Book [:Tomcat]]

yeah that sound like something we should do. Thanks for filing Wes.

We should only test when this goes live against the autostaring feature so that this get not blocked or so by this change :)

Comment 3

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Philor mentioned that we should probably relax these requirements for the try repo, so I've added an exception for try repos.


On an unrelated note, should we show an alert if you click it while it's disabled, to make sure people catch that their "empty" classification didn't get submitted?

Comment 4

[Cameron Dawson [:camd]]

This sounds fine by me.  I think sheriff buy-in is the critical point to this.  But these limitations are probably great to prevent fat-fingering something and saving a classification while you're in the middle of it.

I wonder if elm, ash, etc would also want that constraint relaxed?  Maybe the list of repos that should have more restrictions is shorter than those that shouldn't?  Would a whitelist make sense?  OR perhaps just by repo category.  try and project categories are excluded?  Totally up to you.  :)

Comment 5

[Ed Morley [:emorley]]

See also bug 1116372 :-)

Comment 6

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Comment on attachment 8739308 [details] [review]
[treeherder] KWierso:disablesubmitwithoutanything > mozilla:master

Okay, changed it up so that it allows any repo that's in the "try" group or the "project repositories" group. Everything seems to work fine playing around with it on my gh-pages branch. 

I don't believe those travis ci failures are from me.

Comment 7

[Cameron Dawson [:camd]]

Comment on attachment 8739308 [details] [review]
[treeherder] KWierso:disablesubmitwithoutanything > mozilla:master

Once the tests pass after a rebase, you should be good to merge.

Comment 8

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Done, but I don't have a merge button.

Comment 9

[Treeherder GitHub Bugbot]

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

https://github.com/mozilla/treeherder/commit/f0c818ed8d2e2f9bcba02b5c7f0a809fbb3eb2b7
Bug 1263078 - Don't allow classifications without putting some effort into it

https://github.com/mozilla/treeherder/commit/73b6eed104811ce59542abf9f18dacef3289c38c
Merge pull request #1398 from KWierso/disablesubmitwithoutanything

Bug 1263078 - Don't allow classifications without putting some effort into it