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