[dune-pdelab] Workflow

Jö Fahlke jorrit at jorrit.de
Thu Jun 25 18:23:26 CEST 2015


Am Thu, 25. Jun 2015, 16:35:26 +0200 schrieb Dominic Kempf:
> Date: Thu, 25 Jun 2015 16:35:26 +0200
> From: Dominic Kempf <dominic.r.kempf at gmail.com>
> To: Steffen Müthing <steffen.muething at iwr.uni-heidelberg.de>, dune-pdelab
>  mailing list <dune-pdelab at dune-project.org>
> Subject: Re: [dune-pdelab] Workflow
> 
> I think what Steffen meant by "push your master branch to gitlab" was
> something like
> 
> git push gitlab master:feature/somefeature
> 
> which you can always do.

OK, lets say someone pre-merges master into his branch in that way.  The merge
commit will have a commit message along the lines

  * [featuere/something] merge 'feature/something' into 'master'
  |\
  * | [master] <some commit on master>
  | * <last regular commit on feature/something>
  . .
  . .
  . .

Note that this is not the message associated with the merge request.

Then sometime later I see the merge request, see that it is automatically
mergable and decide to accept it.

- If I do not pay great attention, if I just check the diff to see whether the
  changes seem OK and I trust the requester to have done "make build-test" or
  something similar, I'll just click "accept" and we'll end up with exactly
  the situation we were trying to avoid, only worse.  We'll now have two
  commits that say "merge 'feature/something' into 'master'"

  (A) * [master] <even more commits on master>
      .
      .
      .
  (B) * merge 'feature/something' into 'master'
      |\
  (C) * | <more commits on master>
  (D) | * merge 'feature/something' into 'master'
      |/|
  (E) * | <some commit on master>
  (F) | * <last regular commit on feature/something>
      . .
      . .
      . .

  From this history you will later be easily mislead as to which commit was to
  which branch.

  Typically you don't say in the commit message that you are committing to
  master.  Except in merge commits, because it is in the default commit
  message there.  The commit message of (B) tells me that the parents were on
  master and feature/something, and that the result was on master.  The
  message of commit (D) tells me again that the parents were on master and
  feature/something, and that the result was on master.  From that I would
  conclude that the right thread was master, and the left thread was
  feature/something, and thus commits (C) and (E) were to feature/something
  and commit (F) was to master.  Which is of course totally not what actually
  happened.

- If I do pay attention, and there were no commits to master in the meantime,
  I'll disregard the shiny "accept" button, go to my local pdelab repository,
  fetch, fast-forward master to feature/something, and push, and if I'm lucky
  noone else pushed anything to master in the meantime.  This is, I believe
  the actually intended outcome.

- If I do pay attention and there were commits to master in the meantime, I'll
  write a message to the requester "please remerge".  A day later he maybe
  finds time to do it, and yet a day later I go through any pending merge
  requests and come across his updated merge request again.  Chances are that
  there have been even more commits to master, for instance because that merge
  request was not the first on my list and I have to write another message
  "please remerge".

  If on the other hand the merge request is automatically mergable, I am still
  not in the previous (intended) case, I am in a complication of it which
  still gives great opportunity for error on my part.  Because now the
  requester has either reset feature/something to some commit that is not a
  descendent of the previous value of feature/something.  Or he has created a
  new branch feature/something2, so there are now two branches with very
  similar names.

If I were a requester in that situation, I'd rather rebase my whole branch
onto a newer master and resolve the conflicts that way[1].  And I'd avoid
merges onto my branch altogether -- merging from master is forbidden anyway,
and if any merges were really needed, I could probably get away with rebasing
onto whereever they came from instead.  That way the probability that a merge
request I'll create at some later time will need manual merging is much lower.

[1] Which is what Dominic did.

If instead we decide to allow merges from master the history would look like
this

  * [master] <even more commits on master>
  .
  .
  .
  * merge 'feature/something' into 'master'
  |\
  * | <more commits on master>
  | * merge 'master' into 'feature/something'
  |/|
  * | <some commit on master>
  | * <last regular commit on feature/something>
  . .
  . .
  . .

This is in my opinion the most natural thing to do, given our current tools.
And the history is recording what has actually been happening, and is less
likely to be misunderstood.

Regards,
Jö.

> On Thu, Jun 25, 2015 at 4:19 PM, Jö Fahlke <jorrit at jorrit.de> wrote:
> 
> > Am Thu, 25. Jun 2015, 15:20:11 +0200 schrieb Steffen Müthing:
> > > after discussing this with Dominic, I think this resulted from a little
> > mixup because he
> > > accidentally did his changes on his local master branch and directly
> > pushed that master
> > > branch to a remote feature branch, causing some confusion when trying to
> > resolve the
> > > manual merge.
> > >
> > > That said, I think that merging master into a feature branch should be a
> > *last resort* measure;
> > > normally, you want to do the exact opposite, i.e. merge feature branches
> > *into* master.
> >
> > I don't quiet understand what would be so bad about this.  It's not like we
> > have a master and a release branch and you'd accidentially merge master
> > into
> > release that way.  And if we had, we'd need to be extra careful anyway,
> > e.g. when choosing the base of our feature branches.  But anyway, let's go
> > with it for now.
> >
> > > AFAIU, the default workflow should be:
> > >
> > > 1) See whether the automatic merge using the button works. If it does,
> > use it.
> > > 2) If it does NOT work, do the following:
> > >
> > > - Push the latest version of your feature branch to GitLab
> > > - Switch to master - git checkout master
> > > - Update master to the most recent version - git pull
> > > - Merge the feature request without fast forward (that can’t happen
> > anyway in this case;
> > >   otherwise the automatic merge in GitLab would work) - git merge —no-ff
> > feature/my-branch
> > > - IMPORTANT: Make sure to include the line "See merge request
> > !<mr-number>“ into your commit
> > >   message, as well as something like „Fixes #<bug-number>“ if this merge
> > request resolves a bug
> > > - EVEN MORE IMPORTANT: Make sure that your merge hasn’t broken anything
> > by running
> > >   "make build-tests“ as you might have broken something when resolving
> > merge conflicts
> >
> > I didn't know about this target.  It is probably a good idea, and I'll
> > give it
> > a try, but I doubt I can rely on it all that much.  In pdelab many files
> > are
> > probably not checked by this, since we don't really have comprehensive unit
> > tests, and I usually have to go find an example in pdelab-howto instead to
> > do
> > my testing.  And pdelab-howto does not compile as a whole for me, so
> > build-test would probably fail even before any my changes I made.
> >
> > > - Push your master branch with the merge to GitLab - git push
> >
> > Can everyone push to the master currently?  This would not work for anybody
> > who can't.  AFAIK they can do merge requests, and they would be able to
> > merge
> > a newer master into their branch, thus resolving merge conflicts.  But the
> > way
> > you describe it the manual merge has to be done by the one accepting the
> > merge.  The one requesting the merge can do little to help with that, even
> > though he is probably better prepared to do this, since he at least knows
> > the
> > ins and outs of one side of the merge.
> >
> > > - If the verification step took too long and there are new commits on
> > master that keep you from pushing,
> > >   *do not rebase or pull* unless you know exactly what you’re doing (git
> > rebase -p anyone?). Just rewind
> > >   your master (git reset —hard HEAD~), then update (git pull) and then
> > redo the merge. You can enable
> > >   rerere (git config —global rerere.enabled true) so that git remembers
> > your manual conflict resolutions and
> > >   you don’t have to do them a second time.
> >
> > Regards,
> > Jö.
> >
> > --
> > Jorrit (Jö) Fahlke, Institute for Computational und Applied Mathematics,
> > University of Münster, Orleans-Ring 10, D-48149 Münster
> > Tel: +49 251 83 35146 Fax: +49 251 83 32729
> >
> > Das Erststudium soll bis zum berufsqualifizierenden Abschluss
> > gebührenfrei bleiben, also bis zur Erlangung der Taxi-Lizenz.
> > -- Akrützel, Ausgabe vom 16.5.2002 (www.akruetzel.de)
> >
> > _______________________________________________
> > dune-pdelab mailing list
> > dune-pdelab at dune-project.org
> > http://lists.dune-project.org/mailman/listinfo/dune-pdelab
> >
> >

> _______________________________________________
> dune-pdelab mailing list
> dune-pdelab at dune-project.org
> http://lists.dune-project.org/mailman/listinfo/dune-pdelab


-- 
Jorrit (Jö) Fahlke, Institute for Computational und Applied Mathematics,
University of Münster, Orleans-Ring 10, D-48149 Münster
Tel: +49 251 83 35146 Fax: +49 251 83 32729

In the beginning the Universe was created.  This has made a lot of
people very angry and been widely regarded as a bad move.
-- Douglas Adams
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 811 bytes
Desc: Digital signature
URL: <https://lists.dune-project.org/pipermail/dune-pdelab/attachments/20150625/9c712fc9/attachment.sig>


More information about the dune-pdelab mailing list