Re: [admin] Testing and GitHub login names

On 04/23/2013 08:43 AM, Robin Berjon wrote:
> On 22/04/2013 13:12 , James Graham wrote:
>> On Mon, 22 Apr 2013, Arthur Barstow wrote:
>>>> The only thing that we ask is that pull requests not be merged by
>>>> whoever made the request.
>>>
>>> Is this to prevent the `fox guarding the chicken coop`, so to speak?
>>>
>>> If a test facilitator submits tests (i.e. makes a PR) and everyone
>>> that reviews them says they are OK, it seems like the facilitator
>>> should be able to do the merge.
>>
>> Yes, my view is that Robin is trying to enforce the wrong condition
>> here.
>
> No, I'm just operating under different assumptions. As I said before, if
> someone wants to review without having push/merge powers, it's perfectly
> okay. I don't even think we need a convention for it (at this point). I
> do however consider that this is an open project, so that whoever
> reviews tests can be granted push/merge power.
>
> Why? Because the alternative is this: you get an "accepted" comment from
> someone on a PR. Either you trust that person, in which case she could
> have merge powers; or you don't, in which case you have to review the
> review to check that it's okay. Either way, we're better off making that
> decision at the capability assignment level since it only happens once
> per person.

FWIW I'm used to a situation in which the opposite approach is generally 
taken; that is a reviewer is responsible for reviewing, but the 
submitter is responsible for doing the final integration of their 
changes. This has several advantages; it is not uncommon for code to go 
through review only for the submitter themselves to realise that there 
was an uncaught mistake or a piece missing. This prevents the overhead 
of a second review/test cycle just to fixup such an error. It also means 
that the submitter is very clear about what has been integrated and what 
has not.

>> Indeed, there are currently 41 open pull requests and that number is not
>> decreasing. Getting more help with the reviewing is essential. But
>> that's a Hard Problem because reviewing is both difficult and boring.
>
> I would qualify that statement. If you're already pretty good with web
> standards and you wish to improve your understanding to top levels (and
> gain respect from your peers), this is actually a really good thing to
> work on. Or if you're implementing, it's likely a little bit less work
> to review than to write from scratch (and it can make you aware of
> corner cases or problems you hadn't thought of). Put differently, I
> think it can be a lot less boring if you're getting something out of it.

Oh yeah, there are theoretically good reasons that people might want to 
spend time on doing test review. But as yet that doesn't seem to be 
enough to get people actually doing it; so far there have been a handful 
of people reviewing tests (I count 6 in total). Clearly we need to do 
something better here.

Received on Tuesday, 23 April 2013 07:56:10 UTC