Closed
Bug 867752
Opened 11 years ago
Closed 11 years ago
[shipping] don't cut off pushes in signoff view for just pending sign-offs
Categories
(Webtools Graveyard :: Elmo, defect, P1)
Webtools Graveyard
Elmo
Tracking
(Not tracked)
RESOLVED
FIXED
3.2
People
(Reporter: Pike, Assigned: peterbe)
References
Details
Attachments
(1 file)
(deleted),
patch
|
Pike
:
review+
|
Details | Diff | Splinter Review |
In bug 668761, we changed the queries to only show the sign-offs that come up after the latest action, or 10 pushes.
In practice, this isn't great.
If there's a pending sign-off and a fallback, in particular, I miss a bit of history of how things go towards that sign-off.
I propose that we change this to:
max((accepted or 10), pending, rejected) pushes.
Which would be minimal push_date.
One way to achieve that is to get the earliest push_date that's at most 10 pushes back (so that you get the 8th push if there are only 8), and add that to the list.
Putting this into the P1 bucket, because it really makes reviewing sign-offs awkward right now.
Assignee | ||
Comment 1•11 years ago
|
||
I've taken the code we talked about and implemented it.
https://github.com/peterbe/elmo/compare/feature;bug867752-dont-cut-off-pushes-in-signoff-view-for-just-pending-sign-offs
(I hope that makes sense).
The suite fails on one error. Here's the traceback::
======================================================================
FAIL: test_5_pushes_no_fallback (elmo.apps.shipping.tests.test_signoff.SignOffAnnotatedPushesTest)
----------------------------------------------------------------------
Traceback (most recent call last):
File "/Users/peterbe/dev/MOZILLA/ELMO/elmo/apps/shipping/tests/test_signoff.py", line 444, in test_5_pushes_no_fallback
eq_(len(pushes), 1)
File "/Users/peterbe/dev/MOZILLA/ELMO/elmo/vendor/lib/python/nose/tools.py", line 31, in eq_
assert a == b, msg or "%r != %r" % (a, b)
AssertionError: 2 != 1
----------------------------------------------------------------------
Ran 6 tests in 1.499s
So, what happens there is that we go further back in time even if the "cutoff" was not a rejected or pending. Changing `if cutoff_dates:` to `if len(cutoff_dates) > 1:` fixes the first part of the remaining breaking test but naturally breaks the others.
Perhaps I can leave this here and we can talk about the rest on the phone.
Assignee | ||
Comment 2•11 years ago
|
||
By the way, my branch implements this https://etherpad.mozilla.org/Z5cQgogqgY
Assignee | ||
Comment 3•11 years ago
|
||
after our phone discussion.
Attachment #748911 -
Flags: review?(l10n)
Reporter | ||
Comment 4•11 years ago
|
||
Comment on attachment 748911 [details] [diff] [review]
More pushes if not accepted
Review of attachment 748911 [details] [diff] [review]:
-----------------------------------------------------------------
r=djangocon
Attachment #748911 -
Flags: review?(l10n) → review+
Comment 5•11 years ago
|
||
Commit pushed to develop at https://github.com/mozilla/elmo
https://github.com/mozilla/elmo/commit/ae035a0b06de2b4f3984a1532ad287618b97ba4b
fixes bug 867752 - don't cut off pushes in signoff view for just pending sign-offs, r=Pike
Updated•11 years ago
|
Status: NEW → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Reporter | ||
Updated•11 years ago
|
Assignee: nobody → peterbe
Reporter | ||
Updated•11 years ago
|
Target Milestone: --- → 3.3
Reporter | ||
Updated•11 years ago
|
Target Milestone: 3.3 → 3.2
Updated•4 years ago
|
Product: Webtools → Webtools Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•