On Sat, 10 Oct 2015 16:44:45 +0200 hasufell <hasuf...@gentoo.org> wrote:
> On 10/10/2015 04:27 PM, Alexis Ballier wrote: > >> The side goal is to review current Gentoo commits for major QA > >> violations and other issues, aiming at improving the quality of > >> ebuilds in Gentoo and helping other developers using bash, ebuilds > >> and git effectively. > > > > This is completely unrelated: since we've had gentoo-commits ml, > > every one has been able to do commit reviews easily, and most devs > > have done so. Self-proclamed reviewers project certainly does not > > have the monopoly of best practices nor perfect knowledge. I hope > > they do keep the monopoly of being harassing though :) > > > > We are not a subproject of the QA team and have no hats to throw > around. Nothing we say is a "you must do this" statement. Only QA can > do that. It is no secret that I don't care about "hats" :) If someone is right, he's right, a QA hat doesn't make something wrong magically right. Also, if you'd ask me, QA should be more about Quality Assurance, meaning training people, writing docs, fixing trivial stuff, helping devs to improve; which implies reviewer project fits perfectly. "you must do this" statements shouldn't even be needed, and are completely useless in a volunteer-based project anyway :) > This is just a concept of peer-reviewing, which was very difficult in > CVS times. I fail to see how post-commit reviews are made easier with git. [...] > > Also, you should probably focus on what's really important: reviews > > like "this is weird, care to explain?" or stylistic nitpicks are > > just a waste of every one time, meaning more important stuff does > > not get done. > > > > 'has_version' (which you are probably referring to) as a conditional > for sedding headers is more than just "weird" and indicates a serious > build system bug that needs to be addressed properly. It indicates a conditional fix. Just as the code says. Before throwing an email to -dev ml, I would have expected a reviewer to do his homework and try to understand what the condition is, when it will be satisfied, and why this was conditional. There is absolutely nothing wrong about not knowing the answer, but using -dev ml for it is a bit spammy IMHO. Alexis.
