[Mesa-dev] [v3 PATCH 01/10] mesa:Define extension ARB_framebuffer_no_attachments

[Rogovin, Kevin]

> 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