Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:113863
MIME-Version: 1.0
References: <CAF+90c9A6zG-nYzc3NNtWGjN7y5FmwqnYzjq6GSFC5NtRsOT0w@mail.gmail.com>
 <CAGgaK7JywM+aU4YTK3tUr3xjN5746LDoda4SC_=A+msGdWhNyg@mail.gmail.com>
 <6c3d7c13-d7cd-c1cc-5876-2b8c200a017c@gmail.com> <CAGgaK7JxmLexnkrt+kLfV47wi=KoeqhnzGjTyrcAQtxemKoSKQ@mail.gmail.com>
 <E344AED6-17BA-4FC9-9173-17F524C40766@php.net> <A7687BC9-9457-4E54-93CC-289F95F30EF6@newclarity.net>
 <CAEKnhAGgrX9467Rku98cLyC9CM+cwGwWd3dtaQvS_jjebH1dmA@mail.gmail.com>
In-Reply-To: <CAEKnhAGgrX9467Rku98cLyC9CM+cwGwWd3dtaQvS_jjebH1dmA@mail.gmail.com>
Date: Tue, 30 Mar 2021 13:29:07 +0100
Message-ID: <CAEKnhAENADeVQTyWASi-i7eoXa7iZ6Uo-bo+omNDgF3Wnb8TLQ@mail.gmail.com>
To: Mike Schinkel <mike@newclarity.net>
Cc: Derick Rethans <derick@php.net>, PHP Internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000a446cc05bec025fa"
Subject: Re: [PHP-DEV] Changes to Git commit workflow
From: bukka@php.net (Jakub Zelenka)

--000000000000a446cc05bec025fa
Content-Type: text/plain; charset="UTF-8"

On Tue, Mar 30, 2021 at 1:21 PM Jakub Zelenka <bukka@php.net> wrote:

>
>
> On Tue, Mar 30, 2021 at 12:47 PM Mike Schinkel <mike@newclarity.net>
> wrote:
>
>>
>>
>> > On Mar 30, 2021, at 5:51 AM, Derick Rethans <derick@php.net> wrote:
>> >
>> > On 30 March 2021 10:43:41 BST, Max Semenik <maxsem.wiki@gmail.com>
>> wrote:
>> >> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev
>> >> <smalyshev@gmail.com>
>> >> wrote:
>> >>
>> >>> Hi!
>> >>>
>> >>> On 3/29/21 4:49 AM, Max Semenik wrote:
>> >>>> Would it also make sense if direct pushes (bypassing the pull
>> >> requests
>> >>>> system) were disallowed completely? I can see multiple problems
>> >> with
>> >>> direct
>> >>>> pushes:
>> >>>
>> >>> This is possible. In fact, there are Git bots that make it easier
>> >> (e.g.
>> >>> prow: https://github.com/kubernetes/test-infra/tree/master/prow) - I
>> >> am
>> >>> using such system over Github at my $DAYJOB and it's generally
>> >> working
>> >>> well. It even has its own built-in karma-like system. However, it has
>> >>> some downsides, as the experience shows:
>> >>>
>> >>> 1. Quick management patches, typofixes, release management patches,
>> >> etc.
>> >>> become more high friction processes.
>> >>> 2. Setup and configuration of such system involves some time
>> >> investment
>> >>> by some knowledgeable people, and it has certain learning curve
>> >> (though
>> >>> once it is set up, it's pretty easy to use).
>> >>> 3. Somebody knowledgeable needs to maintain it, as periodically bots
>> >> can
>> >>> get stuck and need a gentle kick to continue.
>> >>> 4. CI needs to be very stable and clean for having CI pass as gateway
>> >> to
>> >>> merge, otherwise a flaky test can block all work in the repo for
>> >> days.
>> >>> 5. Managing multiple active branches can be a pain.
>> >>>
>> >>> None of these are critical, and we could start small and build it
>> >>> incrementally, of course.
>> >>>
>> >>
>> >> We don't even have to use bots - GitHub allows you to require passing
>> >> CI
>> >> and/or approving reviews to merge.
>> >
>> > How well does that work for merging up fixes from an older bug fix
>> branch up through PHP 7.4, PHP 8.0, and then into master?
>> >
>> > Or for things like new timezone definitions, which is now automated,
>> and would then require a pointless PR?
>>
>> Accepting some PRs can be automated.  Repos can be protects on Github via
>> per-branch rules[1] where permissions and requirements can be assigned.
>>
>> PRs are actually a foundational component of GitOps[2] which is an
>> emerging best practice for managing infrastructure. It was initially for
>> Kubernetes deployments but has become recognized as being beneficial[3] for
>> automating software CI/CD[4] and other workflows.
>>
>>
> The problem is that this is not gonna easily work with the current PHP
> workflow and merging changes up. For example currently if you merge PR to
> PHP-7.4 and then you merge PHP-7.4 to PHP-8.0, you get most of the time
> conflict in NEWS that needs to be manually resolved. We would have to make
> the bot that is able to resolve those conflicts or come up with a different
> way how to handle NEWS. However sometimes there are code conflicts as well
> which the bot cannot resolve. I think the only solution would be to stop
> merging the branches up, do PR's always against master and then just
> cherry-pick the commit to lower branches (it could be potentially done by
> bot automatically in some / most cases) but again that would require some
> changes in the NEWS handling and possibly other things.
>
>
Actually if we needed to do cherry-pick that conflicts, we would still need
PR's for lower branches like having multiple PR's for the same thing that
requires conflicting implementation (cannot be cherry-picked by bot).

Think having everything going through the PR's has certainly benefit as we
could require reviews for each PR which would be certainly good for
security. A similar worklfow is done in OpenSSL for example except it's a
bit different flow there because there are not too many fixes and changes
in lower branches.

Cheers

Jakub

--000000000000a446cc05bec025fa--