Please forgive my point-by-point response. You have some excellent questions and interesting points, and I did not want to loose any context.
On 11/2/10 10:46 AM, Robert Leverington wrote:
I wasn't present in D.C. so can't comment on the arguments made there, but it is my understanding that there are people who are responsible for reviewing code who aren't able/willing to deploy it - so this isn't something that has a binary state.
It's either going to be deployed, and thus needs to be reviewed, merged and scapped or it's not. There's no point to reviewing code that is not going to be merged and scapped, we should never be merging code that has not been reviewed, and we can't scap code that hasn't been merged - reviewed or not. Dividing these tasks is pointless, they either all need to be performed or none of them do.
Also, I think it would be useful to document the movement between review, deployment, and enabling - as even if this is done by a single person they might not be able to complete it in one session, and the transparency is nice.
It is, that's what the state of the revision is for, it lets us keep track of whether the revision has been reviewed, and if so whether it's a good thing to merge or not (ok vs fixme).
Correct me if I'm wrong, but I believe we're talking specifically about completely unreviewed extensions that need looking at entirely - not incrementally. Certainly once an extension has been initially reviewed and deployed, the existing code review system would come in to effect - and I don't think we need to change anything with that at the moment.
If that's true, then we may be over-thinking this. The deployment of new features is only a tiny fraction of the review, merge and scap workload.
- Trevor