[Scummvm-devel] Engine merges

[Paul Gilbert]

Hey all,

As the "man in the middle", so to speak, I'd like to throw in my own
thoughts on the matter. Firstly, as per previous email, I thought I'd
satisfied the requirements for merging, although this seems not to be
strictly the case, and though I would dispute that the Access engine was
merged "right in the middle" of discussion, that there is dissent seems
indicative that the rules may need tightening. The question, which wjp is
rightly saying, is how best to handle future engine merges.

I'd like to throw in a couple of suggestions, particularly since the last
point on the Wiki was subject to my own misunderstanding:
* Increase the minimum time to two weeks. This will give more time for
anybody to review the code. As pointed out, pull requests are a good place
to raise objections, and give constructive criticism on the code, so it
would make sense to give greater leeway for any responses.
* Rather than the current last rule (re approval), which would require
chasing down leads for approval, simply shift final merger duty to any core
team member. That way, with an increased minimum review time, anyone who
wants to comment would rightly be expected to have commented by then; any
member that hasn't even bothered to make a comment by the end of the period
will then automatically assumed to be in favor. And with the core team
member having merged it, they're effectively accepting the engine into
master on behalf of the whole team, and giving the decision that they
believe all active suggestions, corrections, etc. have been resolved.

Paul.

On Fri, Dec 19, 2014 at 7:27 PM, D G Turner <d.g.turner at ntlworld.com> wrote:

> Willem,
>   One minor point on this...
>
>   I have said several times before to various people that the design of
>   Github's Pull Request Review and Merge GUI exacerbates this problem.
>
>   I can't find any note I sent to scummvm-devel on this, but I
>   did say about this at 12:31 on 14th April 2012 and I am sure several
>   other times on IRC:
>   http://logs.scummvm.org/log.php?log=scummvm.log.14Apr2012&format=text
>
>   The issue is that all Pull Requests eventually come down to a single
>   developer with write permissions to the repository, pushing the Merge
>   button, which does not align with a group decision system i.e. all
>   three core developers have to push their own "OK to merge" buttons
>   before the merge occurs.
>
>   The current options for improving this boil down to:
>   1. Restrict repository commit / write permissions to just Core Team.
>      All other people work using their own trees and issue Pull
>      Requests.
>      This is undesirable as it creates a barrier to team members fixing
>      bugs and other small changes and is just generally divisive.
>
>   2. The current situation where all team members have commit / write
>      permissions and thus can merge a Pull Request, so we have to rely
>      on a informal "Chinese Whispers" to check agreement.
>
>   I suggest that we should file a feature request on Github to add the
>   ability to have a "sign-off" list on Pull Requests i.e. the person
>   opening the Pull Request adds developers to a sign-off list when
>   creating the request (or does nothing in which the current behaviour
>   occurs). It is then required that each developer clicks a "Merge
>   OK/Block" button to signal their "vote". When the last developer
>   clicks their state to "OK to merge", they trigger the merge or at
>   least unlock the merge dialog. Any developer can remove themselves
>   from a sign-off list and project admins can add people to a list.
>
>   Of course, since Git is a distributed RCS, it is always possible
>   for any developer with write permission to merge a PR branch manually
>   and push, bypassing this process... but this is not critical as this
>   will be very obvious.
>
>   The point would be to allow "voting" on Pull Requests and avoid this
>   kind of "I didn't say Yes yet!" arguments in future.
>
>   Since I think you have dealt with Github support before (and I can't
>   find the damn tracker for reporting issues currently!), could you
>   file this please and drop a link back with their reponse or a link
>   to the feature request artifact.
> Thanks,
> David Turner
>
>
>
>
>
> ------------------------------------------------------------------------------
> Download BIRT iHub F-Type - The Free Enterprise-Grade BIRT Server
> from Actuate! Instantly Supercharge Your Business Reports and Dashboards
> with Interactivity, Sharing, Native Excel Exports, App Integration & more
> Get technology previously reserved for billion-dollar corporations, FREE
>
> http://pubads.g.doubleclick.net/gampad/clk?id=164703151&iu=/4140/ostg.clktrk
> _______________________________________________
> Scummvm-devel mailing list
> Scummvm-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/scummvm-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.scummvm.org/pipermail/scummvm-devel/attachments/20141219/e8937894/attachment.html>