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.
