> 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

Reply via email to