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.
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.
> 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.
> 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.
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.
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
