[PATCH 04/25] mtd: nand: gpmi-nand: janitorial cleanup: (commas after last element of struct initializer)

[David Woodhouse]

On Thu, 2013-08-01 at 07:50 +0200, Lothar Waßmann wrote:
> Huang Shijie writes:
> > is it any bad influence if we do not apply this patch?
> > 
> > I am not clear what we can benefit from this patch.
> > 
> less eye sore when looking at the code and eventual copies of it in
> other places.

But it's actually a *good* practice to have the redundant commas at the
end of the line, in the general case.

It does absolutely no harm, and makes the final line appear *consistent*
with the other lines. I don't see why it would make your eye sore on the
final line when it's there and necessary on the previous lines.

It's also not as if you're going to miss the closing brace and the fact
that it's the end of the struct. The indentation should always make that
clear regardless of the comma.

If we don't include it, we end up with patches which are less readable,
when we add a field to an initialiser. Instead of this nice and obvious
patch, for example:

@@ 1,4 1,5
 struct wombat = {
 	.somefield = foo,bar
	.otherfield = bar,
+	.newfield = baz,
 };

... we end up with this less obvious and harder-to-read patch:

@@ 1,4 1,5
 struct wombat = {
 	.somefield = foo,
-	.otherfield = bar
+	.otherfield = bar,
+	.newfield = baz
 };



Now, admittedly that's the *general* case, and the ones you've removed
commas from are very unlikely to ever have new members added after them.
Especially the one that's a NULL sentinel at the end of an ID match
list. But I'm just not sure I approve of the concept of removing the
commas. I'm happier with them present, and a general habit of including
them.


-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse at intel.com                              Intel Corporation
-------------- next part --------------
A non-text attachment was scrubbed...
Name: smime.p7s
Type: application/x-pkcs7-signature
Size: 5745 bytes
Desc: not available
URL: <http://lists.infradead.org/pipermail/linux-mtd/attachments/20130830/80ed0e0f/attachment.bin>