On Tue, Mar 30, 2021 at 1:21 PM Jakub Zelenka <bu...@php.net> wrote: > > > On Tue, Mar 30, 2021 at 12:47 PM Mike Schinkel <m...@newclarity.net> > wrote: > >> >> >> > On Mar 30, 2021, at 5:51 AM, Derick Rethans <der...@php.net> wrote: >> > >> > On 30 March 2021 10:43:41 BST, Max Semenik <maxsem.w...@gmail.com> >> wrote: >> >> On Tue, Mar 30, 2021 at 3:29 AM Stanislav Malyshev >> >> <smalys...@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
