On 26/03/11 14:56, Mark A. Hershberger wrote: > So, other than switching to the mythical GIT, where all is rainbows and > roses, what can we do to improve code review now?
It's no mystery. After the 1.17 deployment, the team that was doing code review was disassembled. If you want code review to happen faster, then getting people to work on it would be a good start. > If code is to survive past a week in the repository, it has to be > reviewed. > > If you want to make a commit that depends on un-reviewed code, you have > to find someone to review it. Otherwise, your commit will break trunk > when that code is reverted. Find someone to review it? If the experienced developers on the WMF payroll aren't assigned to code review, then under your proposal, the only option for avoiding a revert will be to get someone with no clue about anything to rubber-stamp the code. However, volunteer developers aren't always the most capable people at navigating bureaucracy. In practice, a lot of people would commit code, have it reverted, and leave. If the code review manpower is there, we can be friendly and encouraging to our developers, not threaten them with a revert unless they can make at least one developer be their friend within seven days. The WMF really is central in this, because we have a policy of hiring as many experienced developers as possible from the volunteer community. So that is where the expertise is concentrated. > FIXMEs would disappear. FIXMEs would be up for reversion almost > immediately. Give the committer a day to fix the code, but if it > survives 24 hours as a FIXME, it gets reverted. By definition, our volunteer developers have lives outside of MediaWiki. We have to fit in with their schedules. I don't think we should give them a kick in the teeth just because they committed something on Sunday and have to go to school on Monday. If a commit is insecure, or changes interfaces in a way that will be disruptive to other developers, or breaks key functionality, then sure, we should revert it right away. There's no need to wait 24 hours. But I don't think we need to be issuing death sentences for typos in comments. A "fixme" status just means that there is something wrong with the commit, however minor, it doesn't mean that any urgent action is required. Your proposal seems to be based on the idea that review under Git is many times better than review with CodeReview and Subversion. I don't think that's true, I think it's very slightly better. Whether you use Git or Subversion, you still need people with brains reading code. -- Tim Starling _______________________________________________ Wikitech-l mailing list Wikitech-l@lists.wikimedia.org https://lists.wikimedia.org/mailman/listinfo/wikitech-l
