Document GitLab as the only way to contribute to Samba?

[Michael Adam]

On 2019-06-21 at 21:28 +1200, Andrew Bartlett via samba-technical wrote:
> On Fri, 2019-06-21 at 19:05 +1000, ronnie sahlberg wrote:
> > I think something like this needs a lot more discussion than "need
> > new
> > process because I don't want to read patches."

This or similar statements have been made a few times in this
thread. I would like to emphasize that this is not at all true!
Thorough patch reading and review has to happen with any tool
and process. But with a tool like gitlab, the reviewer can
chose (and usually choses) to review the patches after the
CI has passed on them. That's what's meant by "known-good".

That alone helps a lot and avoids unnecessary review roundtrips.
(This more automatic part of the review is taken up by the tool.)

But only the human reviewer can approve the patch in the merge
request. And both in gitlab and on the mailing list people can
do thorough or sloppy reviews. This has nothing whatsoever to do
with the tool or process used.

> G'Day Ronnie,
> 
> I do wish to be very clear, this is not a discussion about a new
> process, this is a discussion about formalising our current practice.  

I agree that the state to have two processes in parallel is good
to test a new process, but it is not a sustainable situation over
an extended period of time. So I think it is only fair to propose
deprecation of the original process if the new one is already
the de-facto standard.

> Over the past few months, only Martin and to a lessor extent Christof
> has sent a significant number of patches via the list, the rest go via
> GitLab.  
> 
> I'm sorry you haven't noticed, perhaps you thought we just went quiet!
> 
> > Now, samba is a very active project, and like the linux kernel, a
> > very
> > unusual project in that almost all main contributors are
> > paid to work part or full time on samba,
> > That is not the norm for the average open source projects.
> > 
> > Anytime you add special hoops and gatekeepers to contribute patches
> > you will turn away new contributors.
> > That is fine if you don't see it as an issue if it might turn
> > occasional contributors away.
> > The norm for most open source project IS to send patches to the list
> > and get feedback on them.
> > Even the linux kernel works that way, although it is split into
> > several subsystem specific mailinglists.
> 
> I would dispute that it is the norm, but I guess it depends how you
> measure the norm.  Very many projects only accept contributions by
> GitHub pull requests. 

I agree. It surely depends on what kinds of projects you are
usually working on, but my perception is that the vast majority
of projects nowadays use github, gitlab, gerrit, or a similar gui-driven
platform. It seems to me that it generally also lowers the entry
bar for new contributors. (Whether we'd get new contributors because
of switching to gitlab is not guaranteed of course.)

I personally think that mail list reviews do have some advantages
but the gitlab system also has several advantages.

So I would in general be fine with the change.

Not sure if it would help to first declare the ML submissions
deprecated and in a second step declare gitlab the only way to
submit?

Cheers - Michael


> > I think from what you are saying is that the real problem is that
> > contributors send patches to the list but the core
> > developers do not care/ do not want to do patch review, and that is
> > why the patches are ignored and forgotten.
> 
> > If that is the problem, then just changing to a much much harder and
> > different process to  contribute patches
> > is not going to address the problem. I mean, if people can not care
> > to
> > review patches that are sent to the list, why
> > would you think people would review the same patches if they were
> > contributed via a different mechanism?
> 
> As someone who has the great pleasure of reviewing a significant number
> the patches submitted to Samba, I find that GitLab merge requests are
> significantly easier to review because:
>  - The patch, CI results and discussion are all in one place
>  - The original submitter gets the CI feedback automatically, so I
> don't have to tell them it failed
>  - The outstanding patches are in an ordered list that I can work
> though.
>  - I can pull them to my local system, on a branch, rebased on master,
> with simple aliases around wget.
> 
> I do this day in, day out and find it has greatly boosted my
> productivity, and so improved Samba because I'm able to do even more
> code review!  I got 1700 patches reviewed last year, I used to only
> manage 1000.
> 
> Even our contributors praise the use of GitLab, because they too love
> knowing that their patches pass CI, and so are not a embarrassment
> (while not a problem you suffer, first time contributors tend not to be
> very confident). 
> 
> > If the problem is that core contributors do not want to or have time
> > to review patches then the correct solution would
> > probably be to have everyone set aside one day a week to work on
> > project hygiene, and spend that whole day ONLY on working on
> > patch review rather than invent a new system that might stop these
> > patches from being submitted in the first place.
> 
> While I'm incredibly grateful to be in a position to do code review
> actively, on the Samba Team we have not found that berating other
> developers has worked well for, well, anything (frankly).  
> 
> But we now work smarter, rather than harder, and I'm just trying to get
> consensus to update our docs to match our new, better, reality.
> 
> > You even say in a post that you don't care and wont review any
> > patches
> > that are sent to the mailing list.
> > If that is how most core developers think of patch review, maybe that
> > is the actual problem.
> 
> I think you totally miss the point here.  
> 
> Andrew Bartlett
> 
> -- 
> Andrew Bartlett                       https://samba.org/~abartlet/
> Authentication Developer, Samba Team  https://samba.org
> Samba Developer, Catalyst IT          
> https://catalyst.net.nz/services/samba
> 
> 
> 
> 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20190628/982001dc/signature.sig>