1134285 - Hide the Reviews/Diff links for a child commit with no siblings

Status: RESOLVED FIXED

(MozReview Graveyard :: General, defect)

Description

[Matthew N. [:MattN]]

The most common issue I see (now that bug 1097213 is fixed) is that people aren't getting the r+ propagating to BMO because they are reviewing the child commit instead of the parent. I personally think that having a BMO attachment per commit would avoid the confusion but in the short term (before bug 1039679), can we hide the "[Reviews] [Diff]" under the child commit if there is only one commit?

Comment 1

[Matthew N. [:MattN]]

Attachment: MozReview Request: bz://1134285/MattN

/r/3993 - rbmozui: Hide the reviews/diff links on the child review request if there is only one. (bug 1134285)

Pull down this commit:

hg pull review -r bad7aabd933aec404819acbde9a3f2e51b85e4ca

Comment 2

[Matthew N. [:MattN]]

https://reviewboard.mozilla.org/r/3991/#review3149

::: pylib/rbmozui/rbmozui/static/css/review.less
(Diff revision 1)
> +#rbmozui-commits-children > li:first-of-type:last-of-type > div:last-child {

I realize "div:last-child" is somewhat fragile as-is but this is just a quick hack to stop people from getting burned by reviewing the only child review.

Comment 3

[Mike Conley (:mconley) (:⚙️)]

https://reviewboard.mozilla.org/r/3991/#review3153

> I realize "div:last-child" is somewhat fragile as-is but this is just a quick hack to stop people from getting burned by reviewing the only child review.

That's really snazzy - I've never seen the first-of-type:last-of-type hack before. Nicely done. :)

Comment 4

[Mike Conley (:mconley) (:⚙️)]

Comment on attachment 8566100 [details]
MozReview Request: bz://1134285/MattN

https://reviewboard.mozilla.org/r/3991/#review3155

Ship It!

Comment 5

[Matthew N. [:MattN]]

Thanks!

http://hg.mozilla.org/hgcustom/version-control-tools/rev/5cbcd94a3995

Comment 6

[Gregory Szorc [:gps]]

https://reviewboard.mozilla.org/r/3991/#review3183

I don't feel this is an ideal solution and I wouldn't be surprised if we drop this change in the next few weeks or months.

The main problem with this change is it assumes the review series will only ever be a single commit. If we add another commit, we'll presumably have a bunch of review comments on the parent review request instead of the child. I don't think that is proper.

I think we should be encouraging activity to occur on the review request associated with the commit and the parent should be de-emphasized. But until something better comes along, I'm fine with this change, as it sucks less than before and should result in less confusion. I can't wait to have better integration between Bugzilla and multi-commit reviews.

Comment 7

[Mark Côté [:mcote]]

Note that this was backed out in bug 1141588 because it caused too much confusion.

Comment 9

[Matthew N. [:MattN]]

Attachment: MozReview Request: rbmozui: Hide the reviews/diff links on the child review request if there is only one. (bug 1134285)