Consider changing the wording when a review requires fixes

REOPENED
Unassigned

Status

MozReview
Review Board: User Interface
REOPENED
2 years ago
a year ago

People

(Reporter: erahm, Unassigned)

Tracking

Details

Attachments

(2 obsolete attachments)

Currently there's a comment with my face shouting 'Fix it!' if I flagged an issue to be dealt with (even if I gave the review an r+). As a reviewer I try to make all of my comments constructive and positive, I'd prefer that my avatar convey that same tone.
Maybe something like this instead?

http://i.imgur.com/gA42Rc7.png
(In reply to Mike Conley (:mconley) - (needinfo me!) from comment #1)
> Maybe something like this instead?
> 
> http://i.imgur.com/gA42Rc7.png

That seems reasonable to me.
Comment hidden (mozreview-request)
Comment hidden (mozreview-request)
sorry about doing this, but this bug sparked quite a discussion in the mozreview team about if this change should happen.
the final consensus is that we will leave the existing text as-is.

the text needs to clearly and concisely convey that action is required by the patch author before it will be good to land, something that "fix it" achieves.
Status: NEW → RESOLVED
Last Resolved: a year ago
Resolution: --- → WONTFIX
(In reply to Byron Jones ‹:glob› from comment #5)
> sorry about doing this, but this bug sparked quite a discussion in the
> mozreview team about if this change should happen.
> the final consensus is that we will leave the existing text as-is.
> 
> the text needs to clearly and concisely convey that action is required by
> the patch author before it will be good to land, something that "fix it"
> achieves.

Perhaps you should include your users in the conversation as well, there's a reason this bug was filed.
Attachment #8790330 - Attachment is obsolete: true
Attachment #8790330 - Flags: review?(glob)
Attachment #8790331 - Attachment is obsolete: true
Attachment #8790331 - Flags: review?(glob)
(In reply to Eric Rahm [:erahm] from comment #6)
> Perhaps you should include your users in the conversation as well, there's a
> reason this bug was filed.

The conversation was partly about technical complexity, and partly about clarity.  Feel free to argue your case further; we have been known to change our minds in the past. :)
(In reply to Mark Côté [:mcote] from comment #7)
> (In reply to Eric Rahm [:erahm] from comment #6)
> > Perhaps you should include your users in the conversation as well, there's a
> > reason this bug was filed.
> 
> The conversation was partly about technical complexity, and partly about
> clarity.  Feel free to argue your case further; we have been known to change
> our minds in the past. :)

Okay see comment 0, I don't want to use a tool that makes me seem actively hostile to the person I'm reviewing.
Right, we get that.  Unfortunately this is pretty ingrained in Review Board and would involve a nontrivial amount of work to change.  Is there some way we can measure if this actively turns people off?  I know I've heard similar ideas with "r-" but to my knowledge no one has ever tried to quantify the effects.  As this is the first time I've heard of an issue with "Fix it", I would want to make sure the work involved would be justified (we have quite a number of high-priority UI and usability issues we're working on right now).

To be clear, I'm not saying you are wrong, but we have to balance this with the other 500+ bugs filed against MozReview (in addition to larger feature work).
(In reply to Mark Côté [:mcote] from comment #9)
> Right, we get that.  Unfortunately this is pretty ingrained in Review Board
> and would involve a nontrivial amount of work to change.  Is there some way
> we can measure if this actively turns people off?  I know I've heard similar
> ideas with "r-" but to my knowledge no one has ever tried to quantify the
> effects.  As this is the first time I've heard of an issue with "Fix it", I
> would want to make sure the work involved would be justified (we have quite
> a number of high-priority UI and usability issues we're working on right
> now).
> 
> To be clear, I'm not saying you are wrong, but we have to balance this with
> the other 500+ bugs filed against MozReview (in addition to larger feature
> work).

I don't know the best way for you to prioritize this, perhaps ask in a dev-platform post or maybe ask members of our community focused on bringing in a new users? It's possible you already did, but it was done outside of the scope of this bug so I don't know.

The existing patch indicates it's not a heavy burden to change (a one phrase change if you will), I suppose the real work is getting a reasonable result out of any ensuing bikeshedding.
I know both mhoye and jdm work a lot with new contributors.  Do either of you have opinions on this?  I'm not crazy about "Feedback" because (a) it's not a clear instruction as to the next step like "Fix it" is, and (b) it clashes with the Feedback flag (which we will most likely support in MozReview at some point).  I think it's arguable if "Fix it" is aggressive, but maybe you've heard some feedback and/or have other ideas.
Flags: needinfo?(mhoye)
Flags: needinfo?(josh)
I'm strongly opposed to talking to people who are volunteering their time and effort in the imperative. 

What to do about the patch should be in the comments, as I understand it, and given that we're space-limited to... what, eight or nine characters? Ten maybe? Then "Feedback" seems like a better choice, but if we've got the space "Revision Needed" would also be good. 

How much space do we actually have here?
Flags: needinfo?(mhoye)
(In reply to Mike Hoye [:mhoye] from comment #12)
> What to do about the patch should be in the comments, as I understand it,
> and given that we're space-limited to... what, eight or nine characters? Ten
> maybe? Then "Feedback" seems like a better choice, but if we've got the
> space "Revision Needed" would also be good. 
> 
> How much space do we actually have here?

I think it's pretty flexible; there's usually a lot of space underneath.

I still think "Feedback" would cause more confusion than its worth, given the feedback flag in BMO.  "Revision needed" isn't bad, though not great... but I like that general sentiment.
We could follow github's lead and phrase it as "Requested changes".
Flags: needinfo?(josh)
Reopening this since there has been good discussion.
Status: RESOLVED → REOPENED
Resolution: WONTFIX → ---
You need to log in before you can comment on or make changes to this bug.