My favorites | Sign in
Project Home Downloads Wiki Issues Source
New issue   Search
for
  Advanced search   Search tips   Subscriptions
Issue 1100: All draft comments should be sent out with the review.
49 people starred this issue and may be notified of changes. Back to list
Status:  Released
Owner:  ----
Closed:  May 2015


Sign in to add a comment
 
Reported by vbendeb@chromium.org, Aug 18, 2011
A very common situation: there is change in flight, with several versions in the works. Different comment threads are associated with code in different patchsets. When the reviewer/submitter presses the "publish comments" button, only the draft of the current patchset is published. This often causes unpublished drafts.

It would be more robust if all existing draft comments (on all patchsts) were included in the email. 

Ideally, all comment threads should be promoted to the latest patchset (which of course is not always possible) - this would make the review easier with all comment threads visible on the latest patchset.
May 17, 2012
#2 vfe.sarm...@litl.com
This issue is extremely annoying.  It is not at all obvious how to publish draft comments on earlier patch set (you have to select a file from that patch and then hit 'r' from there to publish your review for that patch set).
Oct 1, 2012
#3 dgarrett@chromium.org
This happens to me all the time, and sometimes upsets reviewers because I didn't reply to them.
Oct 25, 2012
#4 s...@google.com
this issue is so disruptive... Especially when people are reviewing the same patch from different time zones.
please please, fix!
Aug 23, 2013
#5 jrobert...@gmail.com
Side note, once you discover that you have a dozen unpublished comments lingering around, it is a nuisance to (find) delete them.

The crux of this problem is when using side-by-side diff mode, any added replies to comments on the old patch set (on the left) aren't published when publishing the review of the new patch set. It would be more intuitive for all comments to be published.
Aug 23, 2013
#6 dgarrett@chromium.org
I just checked the "Draft Comments" link in Gerrit, and I have outstanding comments from over a year ago because of this.
Sep 16, 2013
#7 gok...@google.com
A quick improvement can be achieved by putting a warning in publish page if there are draft comments for other patch sets.

Eventually would love to see a checkbox so that user can select it to include their comments from other patch sets.

This is a common annoyance in our team as well because every now and then somebody (even long time users) forgets to publish their comments.

It is especially annoying when multiple discussions going on in parallel as it is more likely that one of the replies ends up in previous patch and never gets published.

Another trouble is due to new contributors. They usually don't understand why their comments/replies  doesn't show up. That happened to me in my first weeks and just last week another Googler pinged me asking why his replies are not being published.

I hope you can take a look at the problem.
Dec 11, 2013
#9 w...@ooyala.com
That change mentions that the new ui shows patchsets with unpublished comments, and the new change screen does do that...after you click on the "Revisions" drop down and look for a red speech bubble. Then at least you know something is worng. However, I still think it would be more intuitive to be able to publish all comment at the same time. This is particularly true when someone is addressing comments from multiple prior patchset cycles for a single change.

For example, user A pushes patchset p1 to the change. User B adds adds 2 comments c1 and c2. User A pushes a new patchset p2 and marks c1 (in p1) done. User B adds a comment (c3) to p2, but he doesn't duplicate c2 in p2. User A pushes a new patchset p3 and wants to mark c2 and c3 done. I believe this has to be done as two separate steps right now. Publish done comments for p1. Then publish done comments for p2. This definitely makes gerrit a harder sell since one has to go fishing for unpublished comments.

As an aside, another annoyance is the c2 being published in p1 won't be shown when looking at a diff of say base..p2. You have to explicitly show p1 to see the comments. Shouldn't they move forward in some manner? I know that is a complex topic, but it would certainly make addressing comments incrementally more pleasant.
Jul 23, 2014
#10 halcyon1...@gmail.com
Would love to see some movement on this issue, we run into it constantly where new patchset(s) are uploaded that don't fully address comments that need replying-to on prior patchset(s).  Ultimately this means as a user you have to click through every revision that you've commented on and hit reply, which is a waste of time.  Please make reply commit all draft comments across all patchsets.
Jul 23, 2014
#11 brandon....@gmail.com
Argh - happened to me today.  This is my #1 gripe with Gerrit.  Thanks for making this software!
Jul 25, 2014
Project Member #12 David.Os...@gmail.com
https://gerrit-review.googlesource.com/58834
Status: ChangeUnderReview
Sep 7, 2014
Project Member #13 jrn@google.com
 Issue 2889  has been merged into this issue.
Sep 11, 2014
#14 sadov...@google.com
Just wanted to +1 this issue. It drives me (and other teammates) nuts!
Sep 30, 2014
#15 silvio.h...@gmail.com
Why was this change simply abandoned? It drives people at out projects insane...
Oct 7, 2014
#16 cuteguil...@gmail.com
Thanks god, sarmstr mentioned the hidden, top secret key you have to press to publish a comment ("r") on earlier patch sets. 

I would never have found it otherwise.

I can't believe such a basic function is just hidden away by Gerrit. Please fix it! 


Oct 7, 2014
#17 silvio.h...@gmail.com
I don't undertsand how pressing 'r' (toggling the 'reviewed' flag) helps with publishing the comments on earlier patchsets?
Dec 22, 2014
#18 sadov...@google.com
This just happened to me again -- immensely frustrating.

+1, why was the change abandoned?
Feb 9, 2015
#20 rich...@frankel.tv
+1, I am replying to my first set of comments in Gerrit and I would like to go home but instead I'm sitting here trying to figure out how to send my draft comments to my reviewer.
May 5, 2015
#23 dborowitz@google.com
Fix submitted and changes should be live on gerrit-review.googlesource.com within a few minutes, for those that want to play around.
Status: Submitted
May 5, 2015
#24 vbendeb@chromium.org
Wow, thank you, Dave! I did not believe this day would come :)
May 5, 2015
#25 sadov...@google.com
Now this is what I call product excellence! ;)
May 5, 2015
#26 dgarrett@chromium.org
Excellent, thanks!
Nov 26, 2015
Project Member #30 david.pu...@sonymobile.com
(No comment was entered for this change.)
Labels: FixedIn-2.12
Dec 21, 2015
Project Member #31 david.pu...@sonymobile.com
(No comment was entered for this change.)
Status: Released
Sign in to add a comment

Powered by Google Project Hosting