[PATCH] RFC: CONTRIBUTING: Embrace gitlab

Wed Sep 12 20:47:14 UTC 2018

On Wed, Sep 12, 2018 at 9:33 PM, Rodrigo Vivi <rodrigo.vivi at intel.com> wrote:
> On Wed, Sep 12, 2018 at 08:04:34PM +0200, Daniel Vetter wrote:
>> On Wed, Sep 12, 2018 at 7:50 PM, Sean Paul <sean at poorly.run> wrote:
>> > On Wed, Sep 12, 2018 at 1:34 PM Lucas De Marchi
>> > <lucas.demarchi at intel.com> wrote:
>> >>
>> >> On 09/12/2018 09:46 AM, Daniel Vetter wrote:
>> >> > So Rodrigo just broke the gitlab ci on libdrm, I figured perfect time
>> >> > to start this discussion.
>
> :)
>
>> >> >
>> >> > There's imo 2 reasons to do this:
>> >> >
>> >> > - No more "oops, I broke make check". If we use gitlab merge requests
>> >> >    gitlab CI will test everything, and we can set 2 checkboxes that
>> >> >    disallow direct pushes (i.e. require merge requests), and that all
>> >> >    merge requests must pass CI first.
>
> Nah. I think there are other ways of avoiding this. If this is critical and
> mandatory the "test" should be part of "all".
>
> On my case here the regular compilation worked. And I had built with both
> autotools and meson.
>
> If "test" was part of main build it wouldn't had broken it.
>
>> >> >
>> >> > - maintainer-tools is a small project with a small team and little
>> >> >    activity. Much easier to figure out the details here than in one of
>> >> >    our big projects. And there's lots to figure out, which we need to
>> >> >    be able to explain (and have documented) for our 50+ team, if we
>> >> >    ever want to use gitlab: commandline tools, emacs modes, best
>> >> >    practices for setup, ...
>
> But I'm not telling we shouldn't do the experiment. This case here is a great
> case for us to experiment a different flow and maybe prove we were always
> wrong on denying all past requests we had for use different tools ;)
>
> Well, I still feel the pain in the heart when I read "The project was successfully
> forked"... But I will have to deal with my own feelings and prejudices for
> good of the team.
>
> So,
>
> Acked-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
>
> for the experiment with maintainer-tools repo....
>
>> >>
>> >> talking about figuring out the details:
>> >>
>> >> - What is the merge strategy to be adopted? 1) A merge commit per
>> >> merge-request 2) a linear branch with commits being rebased; 3)
>> >> semi-linear approach (still a rebase, but with a forced merge commit).
>
> I'd vote number 2. Is there a way to force it?

Yeah there 's a config button:
https://gitlab.freedesktop.org/drm/maintainer-tools -> settings ->
general -> merge requests

I just realized that I've already changed that to "fast forward",
probably when I played around with it. There's also a checkbox there
to only allow merging if the CI pipeline passes. Which I also already
checked.

Under ->settings -> repository -> protected branches we can also
disallow direct pushes.

>> There's an option to force fast-forward merges only. Gives you then a
>> button to auto-rebase+merge once CI passes on the rebased version.
>> Pretty much fire&forget merging, but with belts&straps. I think for
>> maintainer-tools that's the right thing.
>>
>> >> - What should we do regarding the a-b, r-b? Is gitlab going to add those
>> >> automatically?
>>
>> No automation, and right now I'd suggest we squash them in and update
>> the merge request. We definitely need to squash them in for the
>> kernel, so this is also one of those thing I'd want to look into
>> automating.
>
> For me, the manual squash in kernel is last case when people gave rv-b
> on irc or "for the whole series take this only instance".
> This is the reason that I insist in using patchwork for applying all patches
> with my local dim pwaq #<patchwork id list>
> Patchwork saves all tags people suggested, reviews and acks automagically.
>
> I wish we could have something standardized on gitlab for that matter.
> But yes, this shouldn't be blocker for the experiment.

Yes for the kernel we really need a bot to automate this. I want to
prototype/test something like this with maintainer-tools. Atm how to
best do that from an infrastructure pov, like at all.
-Daniel

>> >> I would not check the 2 boxes to disallow direct pushes until those are
>> >> figured out.
>> >
>> > Given the size of this repo and the number of contributors, I'm not
>> > convinced either of these should be blockers. We should avoid merge
>> > commits since the volume is low enough that having to rebase should
>> > be quite rare. Reviews and acks will be recorded in the merge request,
>> > which is easily accessed from the UI.
>>
>> Not sure recording review&acks on the merge request is going to fly
>> for kernel. And that's kinda what I want to test-drive here, in 1st
>> gear :-)
>
> One of the reasons that I would prefer avoiding merge commits and keep
> it linear or at least as most linear as possible.
>
> Not a problem for the small maintainers-tools repo, but for every
> other component the self contained patch is better for OSVs to backport
> and have a clean view in general.
>
> Thanks,
> Rodrigo.
>
>> -Daniel
>>
>> >
>> > Sean
>> >
>> >>
>> >> Lucas De Marchi
>> >>
>> >> >
>> >> > To avoid this being an entirely hypothetical discussion, I've gone
>> >> > ahead and created a merge request for this:
>> >> >
>> >> > https://gitlab.freedesktop.org/drm/maintainer-tools/merge_requests/3
>> >> >
>> >> > For keeping up with activity: Go to the main repo, click the alarm
>> >> > icon (which probably says "Global" now), and change the setting to
>> >> > "Watch". That should keep you updated on all issues and merge
>> >> > requests, like with a mailing list.
>> >> >
>> >> > v2: Add link to merge request.
>> >> >
>> >> > Cc: Jani Nikula <jani.nikula at intel.com>
>> >> > Cc: Sean Paul <sean at poorly.run>
>> >> > Cc: Rodrigo Vivi <rodrigo.vivi at intel.com>
>> >> > Cc: Lucas De Marchi <lucas.demarchi at intel.com>
>> >> > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
>> >> > ---
>> >> >   CONTRIBUTING.rst | 18 ++++++++++++++----
>> >> >   1 file changed, 14 insertions(+), 4 deletions(-)
>> >> >
>> >> > diff --git a/CONTRIBUTING.rst b/CONTRIBUTING.rst
>> >> > index c7846e318b7e..636d94e3c0af 100644
>> >> > --- a/CONTRIBUTING.rst
>> >> > +++ b/CONTRIBUTING.rst
>> >> > @@ -1,11 +1,21 @@
>> >> >   CONTRIBUTING
>> >> >   ============
>> >> >
>> >> > -Submit patches, bug reports, and questions for any of the maintainer tools and
>> >> > -documentation to the dim-tools at lists.freedesktop.org mailing list.
>> >> > +Patches should be sent via `GitLab merge requests
>> >> > +<https://docs.gitlab.com/ce/gitlab-basics/add-merge-request.html>`_.
>> >> > +maintainer-tools are hosted on `freedesktop.org's GitLab
>> >> > +<https://gitlab.freedesktop.org/drm/maintainer-tools>`_: in order to submit
>> >> > +code, you should create an account on this GitLab instance, fork the core Weston
>> >> > +repository, push your changes to a branch in your new repository, and then
>> >> > +submit these patches for review through a merge request.
>> >> >
>> >> > -Please make sure your patches pass the build and self tests by running::
>> >> > +Gitlab CI will automatically run all tests. You can test your patches locally by
>> >> > +running::
>> >> >
>> >> >     $ make check
>> >> >
>> >> > -Push the patches once you have an ack from maintainers (Jani/Daniel).
>> >> > +All merge requests need an ack from at least one of the committers before it can
>> >> > +be pushed. Don't push to master directly.
>> >> > +
>> >> > +Bug reports, suggestions for improvements and questions for any of the
>> >> > +maintainer tools and documentation should be filed as new issues.
>> >> >
>>
>>
>>
>> --
>> Daniel Vetter
>> Software Engineer, Intel Corporation
>> +41 (0) 79 365 57 48 - http://blog.ffwll.ch
>> _______________________________________________
>> dim-tools mailing list
>> dim-tools at lists.freedesktop.org
>> https://lists.freedesktop.org/mailman/listinfo/dim-tools

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch