The goal of this issue is to note in the guide "Contributing to piwik core" at http://developer.piwik.org/guides/contributing-to-piwik-core that we use Pre-merge code reviews, that issues can be tagged with Needs review
tag. Maybe we could note in this guide the Rules / Benefits / Pitfalls (see #6916).
Interesting article How to write the perfect pull request? that we could link to, or reuse content from, in our contributing guide
also:
Very interesting: The Pitfalls of Code Review (And How To Fix Them)
In particular this:
I use a three-tier system for code reviews. Comments that begin with “You might…” indicate that I am making a suggestion, based on my experience. The developer can choose to ignore my suggestion or not, based on their experience. “You should…” comments indicate something I feel strongly about. The developer is strongly encouraged to consider the suggestion, but again, I leave this to their discretion.
Comments beginning with “You must…” are not optional. But these kinds of comments are reserved for security vulnerabilities, coding standard violations, obvious bugs, or other serious code problems. These are the only blocking code review comments, meaning that all other code review comments can be ignored, but these must be dealt with to obtain my approval for the patch. Everything else gets a thumbs-up.
--> What do you think of adopting this system for our pre-merge code reviews?