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