Everything

php

Git workflow, PRs and php-cvs ML

php.internals

Matteo Beccati

5 years ago

Hey everybody,

I've noticed a test failing today on my CI and went through my local
php-cvs ML archive to see what could have caused it, without finding a
match. That has happened a few times already and I believe it was always
with some "Closes GH-XXX" commits.

I'm just guessing here, but maybe in those cases the git workflow from
our wiki page wasn't properly followed when merging pull requests?

https://wiki.php.net/vcs/gitworkflow

Today's example:
https://github.com/php/php-src/pull/5803


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Nikita Popov

5 years ago

On Wed, Sep 16, 2020 at 12:44 PM Matteo Beccati <php@beccati.com> wrote:

> Hey everybody,
>
> I've noticed a test failing today on my CI and went through my local
> php-cvs ML archive to see what could have caused it, without finding a
> match. That has happened a few times already and I believe it was always
> with some "Closes GH-XXX" commits.
>
> I'm just guessing here, but maybe in those cases the git workflow from
> our wiki page wasn't properly followed when merging pull requests?
>
> https://wiki.php.net/vcs/gitworkflow
>
> Today's example:
> https://github.com/php/php-src/pull/5803
>

No, this just means that your CI is running tests that our CI doesn't. In
this case, you are probably using a different MySQL version and thus run a
different subset of tests.

Nikita

Matteo Beccati

5 years ago

Hi Nikita,

On 16/09/2020 12:46, Nikita Popov wrote:

> No, this just means that your CI is running tests that our CI doesn't.
> In this case, you are probably using a different MySQL version and thus
> run a different subset of tests.

I know that, and that's why I keep it running it ;-)

I was just saying that some commits aren't getting to the php-cvs
mailing list, and often that happens when closing GitHub PRs. And I
suspect it has somethinig to do with the way the PR merge is handled.


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Nikita Popov

5 years ago

On Wed, Sep 16, 2020 at 1:20 PM Matteo Beccati <php@beccati.com> wrote:

> Hi Nikita,
>
> On 16/09/2020 12:46, Nikita Popov wrote:
> > No, this just means that your CI is running tests that our CI doesn't.
> > In this case, you are probably using a different MySQL version and thus
> > run a different subset of tests.
>
> I know that, and that's why I keep it running it ;-)
>
> I was just saying that some commits aren't getting to the php-cvs
> mailing list, and often that happens when closing GitHub PRs. And I
> suspect it has somethinig to do with the way the PR merge is handled.
>

Oh sorry, completely misunderstood what you were referring to. I believe
this is related to the size of the diff. Large diffs probably run into some
size limit and don't reach the php-cvs list.

Nikita

Matteo Beccati

5 years ago

Hi Nikita,

On 17/09/2020 20:12, Nikita Popov wrote:

> Oh sorry, completely misunderstood what you were referring to. I believe
> this is related to the size of the diff. Large diffs probably run into
> some size limit and don't reach the php-cvs list.

I don't know to be honest. From a quick look when the diff is >16kb, an
attachement is generated instead (vs inline diff) and in my archive I
have emails hitting 250kb.


Cheers

--
Matteo Beccati

Development & Consulting - http://www.beccati.com/

Remi Collet

5 years ago

Le 17/09/2020 à 20:12, Nikita Popov a écrit :

> On Wed, Sep 16, 2020 at 1:20 PM Matteo Beccati <php@beccati.com> wrote:
>
>> Hi Nikita,
>>
>> On 16/09/2020 12:46, Nikita Popov wrote:
>>> No, this just means that your CI is running tests that our CI doesn't.
>>> In this case, you are probably using a different MySQL version and thus
>>> run a different subset of tests.
>>
>> I know that, and that's why I keep it running it ;-)
>>
>> I was just saying that some commits aren't getting to the php-cvs
>> mailing list, and often that happens when closing GitHub PRs. And I
>> suspect it has somethinig to do with the way the PR merge is handled.
>>
>
> Oh sorry, completely misunderstood what you were referring to. I believe
> this is related to the size of the diff. Large diffs probably run into some
> size limit and don't reach the php-cvs list.

I think this may be related with commit date

When "merging" a PR, initial commit date is preserved and
could be far in the past (and seems to be ignored by notification)

When using "git am", historic seems better
(and we also have both author/commiter names)


Remi