On Wed, Mar 26, 2014 at 01:04:47PM -0500, Ted Gould wrote:
On Wed, 2014-03-26 at 14:56 +1100, Nathan Hurst wrote:
For fixing code, there should be some sort of timer which says that is code is in trunk via the process set up by the project, then it should be fixed if found defective by the original developer if found within 2 or maybe 6 months. Otherwise the code should be considered divorced from it's original writer.
As for policy regarding commit access; we don't do enough to encourage code review. Anything to encourage code review would be great, I don't even mind if I were downgraded to B level 'code must be reviewed before merge' status. Because really, it should be.
I like this. I'll even put my hand up to do some code reviews. Can we just turn code reviews on with our current tooling?
The tooling is there, Launchpad supports this easily, but I think the social aspect is more difficult than the tooling.
Personally I'd prefer to not have "A" or "B" level developers, instead say everyone has to get their code reviewed by someone else. I think that usually makes a significant dent in making someone accountable.
I agree, I should have deleted that part from my reply. I was only agreeing with the code review part.
I'd also suggest that if we're making a change like this we move to a tool as being the only one with permission to land, at least at first. Otherwise people will just commit to trunk as they've always done. It seems like after a release would be a good time to make such a change.
In some ways this is exactly what we have for initial committers - get two patches accepted. I think it would be a nice step to say that anyone can approve, but it must be approved.
njh