As popular as the github workflow is, there are some things I'm not a big fan of. For one, the review tends to cause a flurry of separate notifications as opposed to a single atomic set of comments.
On Mon, Jan 26, 2015 at 6:17 PM, Arpit Agarwal <aagar...@hortonworks.com> wrote: > I think someone mentioned this earlier - the concern was keeping the review > comments searchable from one location, ideally within Jira. > > > On Mon, Jan 26, 2015 at 5:52 PM, Ravi Prakash <ravi...@ymail.com> wrote: > > > Just out of left field: Since we already migrated to git (Thanks everyone > > for that effort) should we try using github's UI? Isn't that how the > > majority of the rest of the world started doing code reviews? > > > > > > On Monday, January 26, 2015 3:57 PM, Arpit Agarwal < > > aagar...@hortonworks.com> wrote: > > > > > > Thanks for that data point Chris. > > > > It looks like reviews.apache.org no longer works. hadoop-hdfs-git may be > > pointing to an outdated repository. I'll file a ticket with Infra. > > > > On Mon, Jan 26, 2015 at 2:41 PM, Chris Nauroth <cnaur...@hortonworks.com > > > > wrote: > > > > > reviews.apache.org is backed by Review Board, and I've had a very > > positive > > > experience with that tool on other projects. HADOOP-9629 is a Hadoop > > patch > > > where we decided to go ahead and use it, and I think it helped. AFAIK, > > > there is no rule against using it in Hadoop, but it does have the side > > > effect of splitting part of the conversation out of jira. If Crucible > > can > > > keep all the review notes sync'd with the jira and searchable, then > that > > > would be very interesting. > > > > > > Chris Nauroth > > > Hortonworks > > > http://hortonworks.com/ > > > > > > > > > On Mon, Jan 26, 2015 at 1:54 PM, Arpit Agarwal < > aagar...@hortonworks.com > > > > > > wrote: > > > > > > > IMO the number one improvement would be a web-based review tool. We > > could > > > > evaluate Atlassian Crucible since it claims to integrate well with > > Jira. > > > I > > > > have not tried https://reviews.apache.org/r/new/. > > > > > > > > Some easy improvements that were also raised by others on the private > > > list: > > > > - Encourage contributors to batch related trivial fixes into a single > > > > patch. > > > > - Require more detailed descriptions with non-trivial patch > > > contributions. > > > > For patches that require knowledge of a specific subsystem a > > > > background+design note would be a good start. > > > > - Eliminate CHANGES.txt. This came up on the dev list not too long > ago > > > and > > > > IIRC Allen did a PoC. > > > > > > > > I am not optimistic about Gerrit from past experience. It does help > > gate > > > > checkins and enforce pre-commit checks (good). I did not find it > > > > user-friendly and it will be an additional hurdle for contributors to > > > > understand (bad). > > > > > > > > Andrew, can the community build on your distributed pre-commit work > to > > > make > > > > it production ready? > > > > > > > > Regards, > > > > Arpit > > > > > > > > > > > > On Mon, Jan 26, 2015 at 11:55 AM, Andrew Wang < > > andrew.w...@cloudera.com> > > > > wrote: > > > > > > > > > Let's move this over to common-dev@, general@ is normally used for > > > > project > > > > > announcements rather than discussion topics. > > > > > > > > > > I'd like to summarize a few things mentioned on the private@ > thread, > > > > > related to streamlining the code submission process. > > > > > > > > > > - Gerrit was brought up again, as it has in the past, as something > > that > > > > > could make the actual process of reviewing and committing a lot > > easier. > > > > > This would be especially helpful for small patches, where the > > mechanics > > > > of > > > > > committing can take longer than reviewing the patch. > > > > > - There were also concerns about forking discussions between JIRA > and > > > > > Gerrit. This has been an issue in Spark, and we'd like to keep > > > > discussions > > > > > and issue tracking centralized. > > > > > > > > > > - Some talk about how to improve precommit. Right now it takes > hours > > to > > > > run > > > > > the unit tests, which slows down patch iterations. One solution is > > > > running > > > > > tests in parallel (and even distributed). Previous distributed > > > > experiments > > > > > have done a full unit test run in a couple minutes, but it'd be a > > fair > > > > > amount of work to actually make this production ready. > > > > > - Also mention of putting in place more linting and static > analysis. > > > > > Automating this will save reviewer time. > > > > > > > > > > Best, > > > > > Andrew > > > > > > > > > > On Mon, Jan 26, 2015 at 9:16 AM, Ted Yu <yuzhih...@gmail.com> > wrote: > > > > > > > > > > > In some cases, contributor responded to review comments and > > attached > > > > > > patches addressing the comments. > > > > > > > > > > > > Later on, there was simply no response to the latest patch - even > > > with > > > > > > follow-on ping. > > > > > > > > > > > > I wish this aspect can be improved. > > > > > > > > > > > > Cheers > > > > > > > > > > > > On Sun, Jan 25, 2015 at 6:03 PM, Tsz Wo (Nicholas), Sze < > > > > > > s29752-hadoopgene...@yahoo.com.invalid> wrote: > > > > > > > > > > > > > Hi contributors, > > > > > > > I would like to (re)start a discussion regrading to our patch > > > review > > > > > > > process. A similar discussion has been happened in a the > hadoop > > > > > private > > > > > > > mailing list, which is inappropriate. > > > > > > > Here is the problem:The patch available queues become longer > and > > > > > longer. > > > > > > > It seems that we never can catch up. There are patches sitting > > in > > > > the > > > > > > > queues for years. How could we speed up? > > > > > > > Regrads,Tsz-Wo > > > > > > > > > > > > > > > > > > > > > > > > > > -- > > > > CONFIDENTIALITY NOTICE > > > > NOTICE: This message is intended for the use of the individual or > > entity > > > to > > > > which it is addressed and may contain information that is > confidential, > > > > privileged and exempt from disclosure under applicable law. If the > > reader > > > > of this message is not the intended recipient, you are hereby > notified > > > that > > > > any printing, copying, dissemination, distribution, disclosure or > > > > forwarding of this communication is strictly prohibited. If you have > > > > received this communication in error, please contact the sender > > > immediately > > > > and delete it from your system. Thank You. > > > > > > > > > > -- > > > CONFIDENTIALITY NOTICE > > > NOTICE: This message is intended for the use of the individual or > entity > > to > > > which it is addressed and may contain information that is confidential, > > > privileged and exempt from disclosure under applicable law. If the > reader > > > of this message is not the intended recipient, you are hereby notified > > that > > > any printing, copying, dissemination, distribution, disclosure or > > > forwarding of this communication is strictly prohibited. If you have > > > received this communication in error, please contact the sender > > immediately > > > and delete it from your system. Thank You. > > > > > > > -- > > CONFIDENTIALITY NOTICE > > NOTICE: This message is intended for the use of the individual or entity > to > > which it is addressed and may contain information that is confidential, > > privileged and exempt from disclosure under applicable law. If the reader > > of this message is not the intended recipient, you are hereby notified > that > > any printing, copying, dissemination, distribution, disclosure or > > forwarding of this communication is strictly prohibited. If you have > > received this communication in error, please contact the sender > immediately > > and delete it from your system. Thank You. > > > > > > > > > > -- > CONFIDENTIALITY NOTICE > NOTICE: This message is intended for the use of the individual or entity to > which it is addressed and may contain information that is confidential, > privileged and exempt from disclosure under applicable law. If the reader > of this message is not the intended recipient, you are hereby notified that > any printing, copying, dissemination, distribution, disclosure or > forwarding of this communication is strictly prohibited. If you have > received this communication in error, please contact the sender immediately > and delete it from your system. Thank You. >
