Hello, the big problem I have with Code Review and the discussion is: - merely testing and confirming newly committed code for correctness (if it works, that it does not break anything else) seems to be possible or at least not too difficult for the majority of us including me - mere checks if the code fulfils code style guides and coding convention seems also be possible for the mass - the amount of bugs requires daily work to fix current priority appears to be fixing bugs (not reviewing fixes) - but to check against security holes, flaws and potentially dangerous side effects seems to be difficult for the average coding person and reviewer Perhaps some of you (as the "profs") could hold a kind of a "(virtual) teaching class" to teach newbies in improving their CR skills? and to have at the end more entrusted code reviewers? "Real teaching classes" could be also done during next Hackathons, or conferences, as lessons and as workshops! Well, it takes efforts and costs at least time: I admit, and we all have other task to do. Looking forward to your answers, CU Tom

Στις 01-06-2011, ημέρα Τετ, και ώρα 15:58 -0600, ο/η bawolff έγραψε: One question is how thorough is thorough: if the code is reviewed for security issues and doesn't apparently break things, does it need to get gone over with a fine toothed comb before we accept it for deployment? I would say no, and that we could live with "the commit adheres to our basic coding guidelines, looks like it will generally do what it says (without necessarily having tested every detail personally), and follows general usage patterns (appropriate use of hooks, cacheing, i18n, etc. etc.)". Can we ensure that folks have (now and ongoing) enough available time to review commits to trunk at that level within 72 hours? This means buy-in from reviewers *and from their managers* that they have X time for this and that time will not get reassigned to other priorities. If we can't ensure this, we need to decide on the minimum level of review we *can* get done, or extend the time period, or both. Asking all committers to trunk to guess whether their reviewer can or cannot make the 72-hour window for their commit seems like a recipe for pissing people off. Just my two cents. Ariel P.S. Oh and for the record I would like 4 releases a year ideally, I could live with 3, and I would be pretty unhappy with two. Deployment from trunk should happen a lot more often. I don't know if we can manage weekly, but it's a good target.