> 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.
I was only offering these (github and e-mail) in the spirit of offering everything :-) And actually to encourage actual discussion of what we really need and how to accomplish that. > > 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. The problem with a longer that one week cycle is that we get larger and larger volumes of patches. With the right tools, we can sustain a weekly cycle. Note however that the review cycle of a patch set needs to be understood to not always be a week. Sometimes it will take several weeks of iterations. What I want to work on though is responsiveness when people submit patches that they get a first review in a timely fashion. Also, with the changeid allowing review comments to be tracked across versions of a patch, I want to encourage posting patches for review earlier so the major features are getting reviewed as they are developed, not one huge review at the end that no one can keep track of. > > 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. We have to do it because once a changeid has been merged, it is marked closed, and can't be resubmitted. This happened because the way we are trying to use gerrithub is non-native and I messed up. This will never happen again (with the usual caveat, never say never). > > 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....) Yes, that is goodness. Long term, we need it to automate testing of a set of temporarily merged patches, so if you and I have submitted patches, we test that they play well together. We also need to automate testing across a wider variety of platforms. > > 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