Hi, As I mentioned in an IRC discussion, my recollection from when OpenAFS implemented gerrit (also their own installation, but not to my knowledge customized), it was basically mandated that developers submit "one patch per change."
>From what I can tell, Frank finds problematic 1) a lack in gerrit of a "changeset" concept, plus, apparently 2) some missing automation to propagate review information into git's history/commit msg I think both are potentially legitimate considerations, with a claim to be related to best practices (so potentially on a par with the motivations to update the review process). In large corporate development environments I've worked in, the aspiration to get the 20% lacking in the off-the-shelf tool would be solved by some internal development. In other cases, someone said, "no discussion, we're using VSS." I don't have a very strong opinion, but I do somewhat feel that however much gerrit helps with some review beset practices, just doing what gerrithub appears to support oob probably brings arbitrary processes, not just best practices. At the same time, I think the objection to "one patch per change" (which appears to be, "this well-formed, logical change which touches many files is too big for me t review") may be overblown. If we could do one patch per-change, objection #1 might be overcome. To overcome objection #2 seems to require some minimal tooling--I'm unclear whether it is compatible with gerrithub; If for some reason not, is there an organization volunteering to host and maintain/co-maintain aprivate gerrit installation? Matt ----- "DENIEL Philippe" <philippe.den...@cea.fr> wrote: > Hi Frank, > > reply is in the message body. I cut some pieces of your original > message > to avoid a too long message where information would be diluted. > > On 04/24/15 23:59, Frank Filz wrote: > > 1. Abandon gerrithub, revert to using github branches for review and > merge. > > This has a few problems. > The issue with github-based reviews are known. Avoiding solving a > current issue by stepping back to formerly known problems may seem > comfortable, but this is no evolution, it's clearly a regression. > > > 2a. One solution here is use an e-mail review system (like the > kernel > > process). > This is prehistory... Kernel community works like this because they > have > this habit since the (Linux) world was new and nothing else existed. > Can > someone seriously say it would be user-friendly ? OK, mail are > archived, > but not indexed. Looking for a review will be like lookingfora needle > > ina haystack. Once hundreds of mail will have been sent and received > and > re-re-re-re-replied finding useful information will become impossible. > > Please don't bring us back to Stone Age. > > > 3. Change our workflow to work with gerrithub better (stop using > the > > incremental patch process). One loss here would be the ability to > bisect and > > hone in on a small set of changes that caused a bug. > It seems like a much better idea. People in my team are developing in > > the Lustre code. The Lustre community works with a private gerrit > since > the beginning. They have their best practices and their workflow. > In particular, they have "Patch windows" : when the window is opened, > > people can submit patchsets. As it closed, people review it, fix > stuff, > rebase code, and the branch is released. No new patchset comes at this > > time. Then the window opens again and a new cycle starts. One > important > point : the "master repo" is the git inside gerrit and no other. This > > means that contributors would only fetch gerrithub to rebase their > work, > github will then become a simple mirror. > Clearly, the "merge/fix/rebase" process is longer than a week. We > could > work this way by simply abandon the one-week cycle we are accustomed > to. > It's just a matter of using new, more adapted, rules and best > practises. > > > 3a. The most extreme option would be to abandon incremental patches. > If you > > have a body of work for submission in a given week, you submit one > patch for > > that work. > Again, I believe that the one-week cycle is the real issue and it's > such > a constraint for release management. You should open/close the > submission window at your will. It would ease your work a lot, > wouldn't > it ? Remember that gerrit was designed to help the release manager, > it's > not designed to be that painful. We may just use the tool the wrong > way. > > > 3c. A process implied by a post Matt found: Perform code review > with > > incremental patches. Once submission is ready, squash the submission > into a > > single patch and get final signoffs on that. Then the single patch > is > > merged. > People can rebase their patchset, even when submitted to gerrit and I > > think they should keep the same changeid. Remember that changeid is > nothing but a mark on a commit to allow gerrit to keep patchset > history. > It's not a commit id. > > > > > If we proceed with gerrit, Malahal and Vincent will need to > re-submit their > > patches with new changeids > From my point of view, changing the changeids would clearly be a > misuse > of gerrit. > > > 1. We need more participation in review and verification on a timely > basis. > Yes. But the timeline can be refine. > > > 2. We should make sure each patch that has significant impact in > areas the > > author may not be able to test is verified by someone who is able to > test > > that area, and then make sure we document that verification in the > review > > history (here is where gerrit COULD shine). > Gerrit can trigger automated tests (as it does with checkpatch.pl). > Github does not (and so do not emails....) > > > > 3. It would be helpful to be able to identify one or two critical > reviewers > > for each patch, and then make sure those people are able to review > the > > patch. > Yes. > > > For those patches that may need more than a couple people to > review, > > we need to stage them at least a week ahead of when we expect them > to be > > merged, and then somehow flag those patches as high priority for > all > > required reviewers to actually review. > The question of timeline comes back again. That's clearly part of the > issue. > > I am, and I always was, a partisan for using gerrit. I will go and > talk > with my Lustre developers and will be back with a summary of the > workflow use for Lustre with their private gerrit. This may be bring a > > new set of information to the current discussion. > > Regards > > Philippe > > ------------------------------------------------------------------------------ > One dashboard for servers and applications across > Physical-Virtual-Cloud > Widest out-of-the-box monitoring support with 50+ applications > Performance metrics, stats and reports that give you Actionable > Insights > Deep dive visibility with transaction tracing using APM Insight. > http://ad.doubleclick.net/ddm/clk/290420510;117567292;y > _______________________________________________ > Nfs-ganesha-devel mailing list > Nfs-ganesha-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel -- Matt Benjamin CohortFS, LLC. 315 West Huron Street, Suite 140A Ann Arbor, Michigan 48103 http://cohortfs.com tel. 734-761-4689 fax. 734-769-8938 cel. 734-216-5309 ------------------------------------------------------------------------------ One dashboard for servers and applications across Physical-Virtual-Cloud Widest out-of-the-box monitoring support with 50+ applications Performance metrics, stats and reports that give you Actionable Insights Deep dive visibility with transaction tracing using APM Insight. http://ad.doubleclick.net/ddm/clk/290420510;117567292;y _______________________________________________ Nfs-ganesha-devel mailing list Nfs-ganesha-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel
