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

[Brian Paul]

On 05/21/2015 05:26 PM, Rogovin, Kevin wrote:
> HI,
>
>
>
>> Or 78 columns, to be safe, but there's exceptions, like if you're
>> defining a big static table/array of info.
>
> Uggg.... I don't mind exceptions, but knowing them is key.
>
>
>
>>>    4. successive parenthesis must have spaces between parenthesis
>
>> Example?
>
> if (some_func(some_argument))
>
> is bad, but
>
> if (some_func(some_argument) )
>
> is good. I am also guessing that
>
> a = foo(bar(x));
>
> is bad and must be changed to
>
> a = foo(bar(x) );

No, I don't think that's our intention.

a = foo(bar(x)); would be my choice.  Seems perfectly readable.

I haven't been following this thread so perhaps I've missed someone 
else's suggestion.


>>>    6. Use "whether condition" when describing a bool instead of "true if condition is true"
>> not sure we care about that.
>
> I hit a nit for it in the series, so someone cared.

Well, we want our comments to be clear, concise and helpful and 
sometimes the best language is a personal preference.  Hard to be 
specific there.


>
>>    7. derived values of structs -should- be prefixed with an underscore (this rule is loaded with exceptions in the code base from its evolution)
>>    8.  "indenting" is 3 spaces, except after switch where it is 0 (but after case it is 3).
>>    9.  open bracket on same line
>
> The 'indent' command in the docs should cover that.
>
>
>>>   11. functions for an extension must check if extension is supported and if not emit an INVALID_OPERATION message instead of relying on function table dispatch to guarantee they are not called.
>
>> Not sure about that, but that's not a coding style standard.
>
> Perhaps coding standard is not the right word, but it is a requirement (atleast it seems that way) and is a trivial requirement to satisfy.
>
>>>    12. (Guessing here) consecutive empty lines are not allowed
>
>> Generally true, except between functions.
>
> Ugg... I hit a nit from an extra space between functions.
>
>>    13. If changing a line that violates the nit rules, fix the line too rather than just adding the change
>
> Yeah, probably.
>
>> Feel free to submit a patch against docs/devinfo.html with this info. :)
>
> I do not mind submitting the patch, but I want to know what the rules are; preferably explicitly written rather than inferred (by me). Especially since I seem to hit nits like no tomorrow even when trying not to :)

I can understand your frustration.  Going around and around with tiny 
changes isn't fun.  But I think we're all interested in getting things 
to look right the first time, rather than having to clean it up later.

Thanks for your patience and persistence.

-Brian