Closed
Bug 1144085
Opened 10 years ago
Closed 10 years ago
Mercurial client incorrectly uses review status field rather than public field to indicate drafts
Categories
(MozReview Graveyard :: General, defect, P1)
MozReview Graveyard
General
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: dminor, Assigned: dminor)
References
Details
Attachments
(1 file, 1 obsolete file)
The client is currently using the review status field to determine whether or not the user needs to publish the review, but the status field is one of "pending", "submitted", "discarded" [1]. We should be using the public field instead [2].
[1] https://github.com/reviewboard/reviewboard/blob/master/reviewboard/webapi/resources/review_request.py#L172
[2] https://github.com/reviewboard/reviewboard/blob/master/reviewboard/webapi/resources/review_request.py#L176
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → dminor
Assignee | ||
Comment 1•10 years ago
|
||
/r/5531 - hgext: use public field rather than status field to indicate drafts (bug 1144085) r=gps
Pull down this commit:
hg pull review -r 0b0024e3efef21dda85d5ac624746f94807ac450
Attachment #8578652 -
Flags: review?(gps)
Updated•10 years ago
|
Attachment #8578652 -
Flags: review?(gps) → review+
Comment 2•10 years ago
|
||
Comment on attachment 8578652 [details]
MozReview Request: bz://1144085/dminor
https://reviewboard.mozilla.org/r/5529/#review4775
This patch is good. But we need to consider how we land and deploy this.
If clients pick up these changes before the server-side changes are fixed, they aren't going to see the new field and they won't show the review request as being in the draft state. That could be a bit confusing.
I'd be in favor of splitting this into 2 patches: 1 that adds the new field to the response payload and another that changes the client to utilize it. We can deploy the first change on the server then land the second patch.
Or we can just not care and deal with the minor UX setback in the window between when this lands and when it is deployed. Which I assume could be made very small if we coordinate...
Assignee | ||
Comment 3•10 years ago
|
||
I've pushed the server-side portion of this patch here [1]. I'll follow up with the client side once that is deployed.
[1] https://hg.mozilla.org/hgcustom/version-control-tools/rev/12df0c1d8184
Status: NEW → ASSIGNED
Assignee | ||
Comment 4•10 years ago
|
||
Pushed the client-side portion of the patch to here: https://hg.mozilla.org/hgcustom/version-control-tools/rev/d526e69430e8
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 5•10 years ago
|
||
Attachment #8578652 -
Attachment is obsolete: true
Attachment #8619782 -
Flags: review+
Assignee | ||
Comment 6•10 years ago
|
||
Updated•9 years ago
|
Product: Developer Services → MozReview
You need to log in
before you can comment on or make changes to this bug.
Description
•