On 03/24/2016 07:03 PM, Thomas Helland wrote:
Welcome to the community!
Some small comments.
We usually write patch messages in present tense.
So, "Format code in .... ".
Some more comments below.
On Mar 25, 2016 01:24, "Rovanion Luckey" <rovanion.luc...@gmail.com
<mailto:rovanion.luc...@gmail.com>> wrote:
>
> This is a tiny housekeeping patch which does the following:
>
> * Replaced tabs with three spaces.
> * Formatted oneline and multiline code comments. Some doxygen
> comments weren't marked as such and some code
comments were marked
> as doxygen comments.
> * Spaces between if- and while-statements and their parenthesis.
>
> As specified on: http://mesa3d.org/devinfo.html#style
>
Nice detailed commit message!
I'm not sure, but I think you should just write:
"According to coding style standards" or something.
Links may die, and so having them in the commit message
may not give us much in the future.
> The only interesting point would be @@ -364,14 +363,9 @@ where the
> following seemingly trivial change is applied.
>
> - boolean destroyed;
> -
> - destroyed = fenced_buffer_remove_locked(fenced_mgr,
fenced_buf);
> + boolean destroyed = fenced_buffer_remove_locked(fenced_mgr,
fenced_buf);
>
> It may be that I'm missing some of the finer points of C making this
> into a semantic change instead of the only syntactic change I was
> after, in which case the change should be removed. It might also be
> that it should be removed from this change set either way since it
> could be considered non-trivial.
>
I'm not sure how this works now, but I believe there was
something with older versions of MSVC that didn't allow
initializing variables like this, and that's why it is separated
in declaration and initialization.
I believe C89 was the thing here? The VMware guys will know.
IIRC, the problem was with named initializers for structures. I don't
think it's an issue now that we require Visual Studio 2013.
Ordinary, simple initializers like above are always fine.
I believe the requirement was lifted to a newer C standard(C99?).
Therefore this is now OK I believe.
I haven't looked to closely at the other changes.
I can do that tomorrow, if no one gets to it before me.
Don't let the minor nitpicking get you down ;-)
There's always the occasional nitpick when you first adapt
to how things are done in the project.
Thanks for your first patch!
Yes.
-Brian
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
