> On 04/28/15 18:13, Frank Filz wrote: > (...) > > 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. > We introduced the 1-week cycle to regulate/sort patches, for we had no > other way of doing it when using github. > I think that gerrit can help regulating patches workflow and so removes the > need for a 1-week cycle. > What we should do could look like this: > - people depose patches and asks people to review it > - gerrit runs automated test on it (like checkpatch) and verify the patches > - do add +1/-1... new version of the patch is done, discussion are made... > and finally the patch is ready > - at this point, people contact the release manager (aka Frank ;-) > ) and ask for landing the patch. > - if the patch is mergeable (fast-forwardable) it is merged (cherry-pick > button in github interface), if not, owner will have to rebase it and restart the > whole review process This can be done on the fly, as patches arrive and are > reviewed. This change a constrained 1-week cycle against several, less > constrained, cycles, one per patch. Keeping this in mind, a single "big and > squashed" > patch is clearly easier to manage. As Matt said, bigger patches are no issue. If > I do a big patch or 10 small ones, all of my changed files will have to be > reviewed, which does have no impact on the workload. In fact, one big patch > is a cool situation : it is easy to rebase, and it depends only on stuff already > pushed and landed. If I publish 5 patches, what if patch 1, 2 and 3 merge fine, > but 4 produce a conflict ? How could I rebase 4 without touch 1,2 and 3 ? This > leads to a dependencies maze, and this is precisely the situation we fell into.
I'm not going to do a rolling merge on a regular basis. Also, the problem with this is that if a bunch of patches are submitted at once, one of them goes in, and then everyone else has to re-submit their patches. And then the next one goes in, and then some more people have to rebase their patches AGAIN. The poor guy at the end of the line has spent his week rebasing his patch... Rolling merge also asks me for more interruption of my workflow to do the merges. Plus, until we have a good automated test, I will have to manually run tests on every patch. When we have a good comprehensive automated test suite, we can talk about relieving the release manager of that responsibility. > Regarding, I believe we should: > - nfs-ganesha in gerrithub (Frank's home in gerrithub) should accept only > fast-forwardable commit (that's a matter of clicking on the right button in the > right page) > - we should provide big and squashed patches, one per feature. For > example, CEA will soon push a rework of FSAL_HPSS, this will be a single > commit. > - the git in gerrit is the reference. Forget githib, at best it a clone to expose > code in a fancy way. This mean that we stop fetch frank's github and we > fetch frank's gerrithub. This is a very important point. It seems to me that if a > patch is landed in gerrithub, the related github repository is automatically > updated. This will prevent Frank from doing not-funny work, getting things > on one side to push it on the other. We use gerrit, gerrit becomes our > reference. That's as simple as that. Forget github or use it to store work-in- > progress stuff of your own. Github is here to stay. This branch: https://github.com/nfs-ganesha/nfs-ganesha/commits/next Will always be the "official" current state of the development branch. I will ALSO push the same to gerrithub, but the nfs-ganesha github is the branch people should look to for tagged weekly merges. And of course: https://github.com/nfs-ganesha/nfs-ganesha/commits/master Is the official current stable release (with additional branches for stable bugfixes). > > 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. > As I said, the "one single 1-week cycle" is replaced by "several independent > patch-related cycle". Some may take weeks, some may take days or less. The > 1-week cycle is github related. If we stop using github as a reference and use > gerrithub instead, we have no need for this 1-week constraint. > > > What I want to work on though is responsiveness when people submit > > patches that they get a first review in a timely fashion. > They can. People could then add stuff in the patch, and squash the result in a > new patch. If the ChangeId is preserved, gerrit will understand that this is a > new version of an already known patchset and keep the same tracks. Currently we are NOT getting responsiveness, we are not getting sufficient review of most patches. This needs to start improving. > > 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. > 10000% agreed ;-) > This is a completely safe way of proceeding. > > > 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). > This is the result of a mistake, and we all are in a learning curve in using > gerrithub. It's no actual issue so far. > I understood that you spoke about modifying changeids in a normal > workflow. Nice to see to do not think this way ;-) As far as possible changeids > are to be preserved. > > > 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. > We do need automated tests. TravisCI can do compilation tests, but it seems > limited to run "client/server" tests on several machines (what > Jenkins/Workflow does or what I do with some of my Sigmund tests). > Ideally, when someone submit a patch, before it is actually merged or even > reviewed, automated test should run on "next+this commit" (a temporary > and volatile branch) to check for correctness. Gerrit can do things like that > (for example it has strong coupling capabilities with Jenkins). The Lustre > community use that kind of feature a lot. In fact review should start only if all > automated tests are OK. Review should start as soon as a patch is available for comment. In fact, if I have a proposed solution that I want discussion of, I should be able to post a patch for review and start collecting review comments. It may well be known it will not run. Also, blocking until all automated tests are OK would stop us dead in the tracks... Results from automated tests need to be looked at by a human, only a human can decide that we can live with a particular failure for now. > In fact, what we need is a common places where we could run tests in an > automated way (something like a "JenkinsHub"...) Dominique is currently in > the process of automating some tests, he says more in another message in > this list. One problem with a common place is that we will never be able to run some of the most important FSALs this way... And actually, I see it likely that there will be more and more proprietary FSALs. But I want to encourage those FSALs to be in tree as much as possible because then we can take them into account when making changes. Of course to be in tree we must be able to compile them without having proprietary libraries. Frank ------------------------------------------------------------------------------ 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
