Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:107259
MIME-Version: 1.0
References: <CAFp73XvOp_y7Gz1F4fdBHt1YiMibJ4zdbjrQ+LFxhHYW+-DoHg@mail.gmail.com>
 <44818c1e-07e4-19d5-2ef5-144ccb2b5c05@gmx.de> <CAFp73XvRoPfBFER3ezGXvhiEM3kTrNN1N-95zi8UC=ksExH_1A@mail.gmail.com>
 <CAAwdEzBufQ11iStNC9TjdF+1dCiaCNYqVB7hUikco7TVH2seyQ@mail.gmail.com>
 <CAF+90c_FpsxjDz9yBzSmZProx9XFxpZymcWXXtFdQEdHe7gEUQ@mail.gmail.com> <CAAwdEzCOGDBjrMDzZKQpwOYMSN93gq=f8G3ViDe2uO_fjEG=UQ@mail.gmail.com>
In-Reply-To: <CAAwdEzCOGDBjrMDzZKQpwOYMSN93gq=f8G3ViDe2uO_fjEG=UQ@mail.gmail.com>
Date: Fri, 20 Sep 2019 12:22:36 +0300
Message-ID: <CAAwdEzAU5ft_PrKfgoAY3J79iTEy8Pn26oKByEtvju4Ur-suwQ@mail.gmail.com>
To: Nikita Popov <nikita.ppv@gmail.com>
Cc: "guilhermeblanco@gmail.com" <guilhermeblanco@gmail.com>, "Christoph M. Becker" <cmbecker69@gmx.de>, 
	PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000f9ca610592f89d1e"
Subject: Re: [PHP-DEV] Re: Question about merged PR #937
From: drealecs@gmail.com (=?UTF-8?Q?Alexandru_P=C4=83tr=C4=83nescu?=)

--000000000000f9ca610592f89d1e
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

I was quick to write an answer and forgot to search for a more detailed
explanation online, being busy with some other work tasks.
Github explains it here:
https://help.github.com/en/articles/about-merge-methods-on-github

Before enabling squashing commits, consider these disadvantages:
>
>    - You lose information about when specific changes were originally
>    made and who authored the squashed commits.
>
>
Alex


On Fri, Sep 20, 2019 at 11:56 AM Alexandru P=C4=83tr=C4=83nescu <drealecs@g=
mail.com>
wrote:

> Hi Nikita,
>
> Yes, you are right that the author (and not the committer) is the one
> taken in considerations for git blame and contributor display. Committer =
is
> just technical and related to who created/recreated a commit.
>
> But, unfortunately, you are wrong in believing that the author is
> preserved during merging with squash. A squash merge is similar with a
> normal commit, it has a single parent and it does not relate with the
> original commits in the git's commit graph.
> I can help with us making a quick test to demonstrate this.
>
> Alex
>
> On Fri, Sep 20, 2019 at 10:36 AM Nikita Popov <nikita.ppv@gmail.com>
> wrote:
>
>> On Fri, Sep 20, 2019 at 12:50 AM Alexandru P=C4=83tr=C4=83nescu <drealec=
s@gmail.com>
>> wrote:
>>
>>> Hi Guilherme,
>>>
>>> That is what is happening when squashing commits during merge.
>>>
>>> I noticed this also few months ago but didn't mentioned it as I though
>>> this
>>> is one of the usual flows.
>>> I quickly searched it now and found it here:
>>> https://wiki.php.net/vcs/gitworkflow: "Additionally, the history of pul=
l
>>> requests often requires cleanup. For most pull requests, all commits ca=
n
>>> be
>>> squashed into one."
>>>
>>> It could be considered a good practice to not pollute git history with =
a
>>> lot of small commits as it would hinder analyzing it later.
>>> On the other hand I quite like (and promote) doing small incremental
>>> steps
>>> during a PR for multiple reasons that I can highlight if needed.
>>>
>>> Analyzing git history could be simplified by using --first-parent when
>>> one
>>> would not want to go in details on second parent of a merge commit with
>>> individual detailed commits.
>>> Also, there is no noticeable git performance improvements when using
>>> squash.
>>>
>>> One of the advantages here would be that author would see his name in
>>> contributors of a file or on repository level and be proud of it.
>>> This is an important factor especially in open source software as
>>> otherwise
>>> it could leave a bitter taste that you are not fully recognized as
>>> contributor in git's history and might decrease the chance to future
>>> contributions.
>>>
>>> Would it be up for discussion to define the way to merge pull requests =
in
>>> order to avoid squashing merges by a different author? Has this issue
>>> been
>>> discussed before?
>>> The mention that "the history of pull requests often requires cleanup" =
is
>>> not clearly defined. I would not agree that PR-937
>>> https://github.com/php/php-src/pull/937/commits or  PR-911
>>> https://github.com/php/php-src/pull/911/commits required squashing.
>>> As a simple solution, if squashing is required it should be done by the
>>> original author, before merging.
>>>
>>> Regards,
>>> Alex
>>>
>>
>> Git has separate notions of "author" and "committer". The author is
>> preserved during squashing, and the author is what is relevant for
>> contribution statistics on GitHub. If the author is preserved but the us=
er
>> doesn't show up as a contributor, that's likely due to a missing mail
>> mapping between the email address and the GitHub account.
>>
>> Nikita
>>
>>
>>> On Thu, Sep 19, 2019 at 8:20 PM guilhermeblanco@gmail.com <
>>> guilhermeblanco@gmail.com> wrote:
>>>
>>> > Thanks for the clarification Christoph!
>>> > Somehow I couldn't see it when looking for the commit hash. Sorry for
>>> > the disturbance in the list here.
>>> >
>>> > PS: I find it funny that even though I'm the original author of the
>>> > code, I don't show as a contributor in any statistics.
>>> > It'd be a shame to not be considered as a valid voter for "lack of
>>> > recent contribution" to the language.
>>> >
>>> > Cheers,
>>> >
>>> >
>>> > On Thu, Sep 19, 2019 at 12:58 PM Christoph M. Becker <
>>> cmbecker69@gmx.de>
>>> > wrote:
>>> > >
>>> > > On 19.09.2019 at 17:01, guilhermeblanco@gmail.com wrote:
>>> > > > One of my old PRs to PHP that was claimed to be merged disappeare=
d
>>> > > > from master. However, the upgrade note is still there in master a=
nd
>>> > > > 7.4beta1.
>>> > > >
>>> > > > Here is the PR: https://github.com/php/php-src/pull/937
>>> > > > Here is the commit referencing it:
>>> > > >
>>> >
>>> https://github.com/php/php-src/commit/0adfa03397edcde8cba3bae2032b1f2ef=
26ea760
>>> > > >
>>> > > > Please let me know if there's anything I can do to help.
>>> > >
>>> > > From what I can tell, everything is there.  PR #911 has been merged
>>> as
>>> > > <
>>> >
>>> http://git.php.net/?p=3Dphp-src.git;a=3Dcommit;h=3D094d409b3d34c51f49e0=
121e5ccfe8b2a717aaf6
>>> > >,
>>> > > and parts of that PR already with PR #928 as
>>> > > <
>>> >
>>> http://git.php.net/?p=3Dphp-src.git;a=3Dcommit;h=3Df48c2327403ce76a870e=
74f31a069a64dfb711a2
>>> > >.
>>> > >
>>> > > --
>>> > > Christoph M. Becker
>>> >
>>> >
>>> >
>>> > --
>>> > Guilherme Blanco
>>> > SVP Technology at Statflo Inc.
>>> > Mobile: +1 647 232 5599
>>> >
>>> > --
>>> > PHP Internals - PHP Runtime Development Mailing List
>>> > To unsubscribe, visit: http://www.php.net/unsub.php
>>> >
>>> >
>>>
>>

--000000000000f9ca610592f89d1e--