Dan Hecht has posted comments on this change. ( http://gerrit.cloudera.org:8080/10827 )
Change subject: IMPALA-7215: Implement a templatized CountingBarrier ...................................................................... Patch Set 4: Code-Review+2 (4 comments) Just a few more comment cleanups for the public interface. http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h File be/src/util/counting-barrier.h: http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@36 PS4, Line 36: returned value Not clear which "returned value" this is talking about - sounds like Notify()'s returned value. The comment for NotifyRemaining is clearer http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@43 PS4, Line 43: unblocks Wait() with : /// the returned value as 'promise_value'. that's clearer. you could say it this way for Notify() comment. http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@56 PS4, Line 56: 'promise_' public comments shouldn't talk about private fields (the client of this class shouldn't care that there is a promise in the implementation). Something like: Returns the value passed to the final Notify() or NotifyRemaining() call. http://gerrit.cloudera.org:8080/#/c/10827/4/be/src/util/counting-barrier.h@61 PS4, Line 61: returns the value set : /// on 'promise_' in Notify() or Notif same -- To view, visit http://gerrit.cloudera.org:8080/10827 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I05fc79228250408ae16481ae7ff3491a90d26b8e Gerrit-Change-Number: 10827 Gerrit-PatchSet: 4 Gerrit-Owner: Sailesh Mukil <sail...@cloudera.com> Gerrit-Reviewer: Dan Hecht <dhe...@cloudera.com> Gerrit-Reviewer: Sailesh Mukil <sail...@cloudera.com> Gerrit-Comment-Date: Thu, 28 Jun 2018 17:35:20 +0000 Gerrit-HasComments: Yes
