> I suppose it could be useful, but I think we've been mostly successful at 
> just expecting people to recognize when what they're writing doesn't look 
> like the code around it.

This is my point. Older code had different style/expectations than newer code. 
For this patch series, I have hit a number of nits and quasi-nits that are 
ambiguous:

0. In v1 to v2, avoid GL types atleast in i965 although much of the code uses 
that (the explanation given was to move away from GL types). Does this apply to 
Mesa core as well?

 1. the casting bit in the i965 patch to use the _mesa_geomety_ functions. 
These are not quite nits, but in some ways not a big deal. I hit these because 
there were ints there before. In this regard doing what was there before was 
ungood (atleast for this review).

2. the line between function thing. In truth I just missed that extra line for 
the added to framebuffer.h (and I should not have), but there are places in the 
code that there are multiple empty lines between function definitions. I do not 
mind saying "no extra empty lines", but not knowing the rules and attempting to 
infer them from the code, I seem to hit too many nits.

3. Even on the subject of git commit, I am seeing different answers: 75, but 
try 50 usually, but understandable if cannot do it. Utterly ambiguous.

4. on the subject of line length I have this so far: usually 78, but 80 
sometimes is ok. Does ok, for example, include making a large-ish comment block 
more justified? 

5. Consecutive empty lines is not ok, except in function definitions, but then 
only sometimes. I am guessing "sometimes" is for grouping function definitions, 
but plenty of files follow different conventions (hence what Brian Paul said).

Given that nits just add traffic (and I want to minimize that silliness) I 
think it would be wise to set down some precise rules so there is no judgement 
calls required for something as silly as formatting nits. Ideally, we would 
have a lint script that would do it for us :) I see that reviews usually first 
hit nits, then content of patches. That is fine, but I'd rather know all the 
rules rather than hitting the nits at all.

Again, I really have no preference since someone is paying me to do this, but 
knowing the exact rules would be more efficient. Inferring rules is quite error 
prone IMO (indeed, at one point, the GLSL pre-processor appeared to be written 
in a different style).

-Kevin
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to