On Wed, Sep 12, 2018 at 9:33 PM, Rodrigo Vivi <[email protected]> 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 <[email protected]> wrote: >> > On Wed, Sep 12, 2018 at 1:34 PM Lucas De Marchi >> > <[email protected]> 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 <[email protected]> > > 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 <[email protected]> >> >> > Cc: Sean Paul <[email protected]> >> >> > Cc: Rodrigo Vivi <[email protected]> >> >> > Cc: Lucas De Marchi <[email protected]> >> >> > Signed-off-by: Daniel Vetter <[email protected]> >> >> > --- >> >> > 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 [email protected] 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 >> [email protected] >> https://lists.freedesktop.org/mailman/listinfo/dim-tools -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ dim-tools mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/dim-tools
