Re: master updated (18e7bc87521 -> c71a520d1da)

Re: master updated (18e7bc87521 -> c71a520d1da)

[Stefan Kangas]

[ Resent with emacs-devel in Cc, sorry. ]

Did you follow the procedure documented in
admin/doc/git-merge-workflow when doing this merge? It looks to me
like you merged master into feature/android and then pushed that to
master, or something along those lines?

I'm seeing that files have their

     git log <filename>

 littered with incorrect (and distracting) commits saying

    Merge remote-tracking branch 'origin/master' into feature/android

This has not happened for any other feature branch (well, except pgtk,
but it is too late to change that).

AFAICT, we see commits like the above when the merge takes place after
the files were first changed on features/android and then [every time
they are changed] on master. In other words, they are displayed on
master now in the way you would expect on feature/android. See "git
log lisp/startup.el" for an example of what it looks like - not good.

Merge commits _from_ master should not be displayed, as they only
serve to make history less readable. That's why git hides them there
by default after you merge said branch to master. On the other hand we
are, of course, interested in when feature/android was merged into
master. But that is _not_ currently visible in "git log <filename>".

I recommend force pushing a correct merge to master before it is too
late to reverse this.

Re: master updated (18e7bc87521 -> c71a520d1da)

[Stefan Kangas]

> Did you follow the procedure documented in
> admin/doc/git-merge-workflow when doing this merge?

Actually, I can't find any correct instructions now.  I thought we had them?

Re: master updated (18e7bc87521 -> c71a520d1da)

[Po Lu]

[Resending with emacs-devel copied in, sorry.]

Stefan Kangas <stefankangas@gmail.com> writes:

> Did you follow the procedure documented in
> admin/doc/git-merge-workflow when doing this merge?

Where is this file, and what does it say?

> It looks to me like you merged master into feature/android and then
> pushed that to master, or something along those lines?

Yes.

> I'm seeing that files have their
>
>      git log <filename>
>
>  littered with incorrect (and distracting) commits saying
>
>     Merge remote-tracking branch 'origin/master' into feature/android
>
> This has not happened for any other feature branch (well, except pgtk,
> but it is too late to change that).

AFAIU, the ChangeLog generator ignores commits made on branches merged
into master, and only refers to the commit message for the merge commit
itself.

> AFAICT, we see commits like the above when the merge takes place after
> the files were first changed on features/android and then [every time
> they are changed] on master. In other words, they are displayed on
> master now in the way you would expect on feature/android. See "git
> log lisp/startup.el" for an example of what it looks like - not good.

How can this be avoided?  I merely ran:

  $ git merge --edit --no-ff feature/android

within a checkout of master; the --no-ff was necessary for supplying a
ChangeLog entry for the merge itself.

> Merge commits _from_ master should not be displayed, as they only
> serve to make history less readable. That's why git hides them there
> by default after you merge said branch to master. On the other hand we
> are, of course, interested in when feature/android was merged into
> master. But that is _not_ currently visible in "git log <filename>".
>
> I recommend force pushing a correct merge to master before it is too
> late to reverse this.

That's totally fine by me, but alas, you have not told me how to perform
this procedure.  (And I can't find a file named git-merge-workflow
anywhere.)

Re: master updated (18e7bc87521 -> c71a520d1da)

[Stefan Kangas]

Po Lu <luangruo@yahoo.com> writes:

> How can this be avoided?  I merely ran:
>
>   $ git merge --edit --no-ff feature/android
>
> within a checkout of master; the --no-ff was necessary for supplying a
> ChangeLog entry for the merge itself.

That looks like the right way to do it, so this is a false alarm on my
end.  Sorry for the ruckus.

I believe that I got confused for a second there by the large number
of merge commits from master into feature/android. I think in the
future it would be beneficial if feature branch authors could try to
keep the number of merges from master a bit lower.

In admin/notes/repo we have this advice:

    In general, when working on some feature in a separate branch, it is
    preferable not to merge from master until you are done with the
    feature.

Perhaps we could expand it with some advice that would be more
directly applicable to long-living and "large" feature branches (such
as android and nativecomp), where postponing the merge until the end
might be impractical.

> That's totally fine by me, but alas, you have not told me how to perform
> this procedure.

Yup, false alarm.  I apologize again.

(I'm pretty jetlagged and just came home after an 11 hour layover due
to a delayed flight, so I should probably not be at a keyboard right
now...)

Re: master updated (18e7bc87521 -> c71a520d1da)

[Po Lu]

Stefan Kangas <stefankangas@gmail.com> writes:

> Yup, false alarm.  I apologize again.
>
> (I'm pretty jetlagged and just came home after an 11 hour layover due
> to a delayed flight, so I should probably not be at a keyboard right
> now...)

No harm done, and all is forgiven.

Re: master updated (18e7bc87521 -> c71a520d1da)

[Robert Pluim]

>>>>> On Mon, 7 Aug 2023 04:58:28 +0200, Stefan Kangas <stefankangas@gmail.com> said:

    Stefan> Po Lu <luangruo@yahoo.com> writes:
    >> How can this be avoided?  I merely ran:
    >> 
    >> $ git merge --edit --no-ff feature/android
    >> 
    >> within a checkout of master; the --no-ff was necessary for supplying a
    >> ChangeLog entry for the merge itself.

    Stefan> That looks like the right way to do it, so this is a false alarm on my
    Stefan> end.  Sorry for the ruckus.

    Stefan> I believe that I got confused for a second there by the large number
    Stefan> of merge commits from master into feature/android. I think in the
    Stefan> future it would be beneficial if feature branch authors could try to
    Stefan> keep the number of merges from master a bit lower.

Like zero :-)

    Stefan> In admin/notes/repo we have this advice:

    Stefan>     In general, when working on some feature in a separate branch, it is
    Stefan>     preferable not to merge from master until you are done with the
    Stefan>     feature.

Which is why you should rebase in that situation, but some people
donʼt like rebase for reasons that I donʼt quite understand. "It was
slightly broken in a version of git from a decade ago" is not a valid
argument: emacs developers should be expected to update their
tools. <duck>

    Stefan> Perhaps we could expand it with some advice that would be more
    Stefan> directly applicable to long-living and "large" feature branches (such
    Stefan> as android and nativecomp), where postponing the merge until the end
    Stefan> might be impractical.

See my previous paragraph.

Robert
-- 

Re: master updated (18e7bc87521 -> c71a520d1da)

[Po Lu]

Robert Pluim <rpluim@gmail.com> writes:

>>>>>> On Mon, 7 Aug 2023 04:58:28 +0200, Stefan Kangas <stefankangas@gmail.com> said:
>
>     Stefan> Po Lu <luangruo@yahoo.com> writes:
>     >> How can this be avoided?  I merely ran:
>     >> 
>     >> $ git merge --edit --no-ff feature/android
>     >> 
>     >> within a checkout of master; the --no-ff was necessary for supplying a
>     >> ChangeLog entry for the merge itself.
>
>     Stefan> That looks like the right way to do it, so this is a false alarm on my
>     Stefan> end.  Sorry for the ruckus.
>
>     Stefan> I believe that I got confused for a second there by the large number
>     Stefan> of merge commits from master into feature/android. I think in the
>     Stefan> future it would be beneficial if feature branch authors could try to
>     Stefan> keep the number of merges from master a bit lower.
>
> Like zero :-)
>
>     Stefan> In admin/notes/repo we have this advice:
>
>     Stefan>     In general, when working on some feature in a separate branch, it is
>     Stefan>     preferable not to merge from master until you are done with the
>     Stefan>     feature.
>
> Which is why you should rebase in that situation, but some people
> donʼt like rebase for reasons that I donʼt quite understand. "It was
> slightly broken in a version of git from a decade ago" is not a valid
> argument: emacs developers should be expected to update their
> tools. <duck>

Rebase isn't possible on feature branches, AFAIK, as they demand force
pushes.

Re: master updated (18e7bc87521 -> c71a520d1da)

[Robert Pluim]

>>>>> On Wed, 09 Aug 2023 16:35:58 +0800, Po Lu <luangruo@yahoo.com> said:
    >> 
    >> Which is why you should rebase in that situation, but some people
    >> donʼt like rebase for reasons that I donʼt quite understand. "It was
    >> slightly broken in a version of git from a decade ago" is not a valid
    >> argument: emacs developers should be expected to update their
    >> tools. <duck>

    Po Lu> Rebase isn't possible on feature branches, AFAIK, as they demand force
    Po Lu> pushes.

You can push a deletion of the feature branch, and then repush the
rebased branch.

Robert
-- 

Re: master updated (18e7bc87521 -> c71a520d1da)

[Po Lu]

Robert Pluim <rpluim@gmail.com> writes:

> You can push a deletion of the feature branch, and then repush the
> rebased branch.

I don't think the 3 or 4 individuals tracking the feature/android branch
would have taken kindly to such a gesture.  (Not merging at all wasn't
an option either, since the branch saw active use.)

Can't the Git people fix this by excluding merge commits from merges
themselves, or something?

Re: master updated (18e7bc87521 -> c71a520d1da)

[Robert Pluim]

>>>>> On Wed, 09 Aug 2023 17:16:14 +0800, Po Lu <luangruo@yahoo.com> said:

    Po Lu> Robert Pluim <rpluim@gmail.com> writes:
    >> You can push a deletion of the feature branch, and then repush the
    >> rebased branch.

    Po Lu> I don't think the 3 or 4 individuals tracking the feature/android branch
    Po Lu> would have taken kindly to such a gesture.  (Not merging at all wasn't
    Po Lu> an option either, since the branch saw active use.)

I wouldnʼt have minded, but then again I wasnʼt making any changes to
the branch either.

    Po Lu> Can't the Git people fix this by excluding merge commits from merges
    Po Lu> themselves, or something?

The git people canʼt but you could have chosen to squash the entire
android branch into a single commit. I donʼt think that would have
been ideal either.

Robert
-- 

Re: master updated (18e7bc87521 -> c71a520d1da)

[Eli Zaretskii]

> From: Robert Pluim <rpluim@gmail.com>
> Cc: Po Lu <luangruo@yahoo.com>,  emacs-devel@gnu.org
> Date: Wed, 09 Aug 2023 10:24:25 +0200
> 
>     Stefan> In admin/notes/repo we have this advice:
> 
>     Stefan>     In general, when working on some feature in a separate branch, it is
>     Stefan>     preferable not to merge from master until you are done with the
>     Stefan>     feature.
> 
> Which is why you should rebase in that situation, but some people
> donʼt like rebase for reasons that I donʼt quite understand. "It was
> slightly broken in a version of git from a decade ago" is not a valid
> argument: emacs developers should be expected to update their
> tools. <duck>
> 
>     Stefan> Perhaps we could expand it with some advice that would be more
>     Stefan> directly applicable to long-living and "large" feature branches (such
>     Stefan> as android and nativecomp), where postponing the merge until the end
>     Stefan> might be impractical.
> 
> See my previous paragraph.

There's no reason to rebase (although if someone wants that, they can
do it).  The preferred workflow for feature branches is to merge from
master only once, just before you are going to land the feature.  Then
test the results of the merge as well as possible, and merge back.

The usual reason that people merge from master is because they are
afraid of master breaking their feature or afraid of merge conflicts.
But these dangers are quite illusory, and if one works on a feature
that is completely new (as in this case), neither of the two dangers
is real.