Re: [PATCH 1/2] x86/quark: Add Quark embedded SRAM support

[Darren Hart]

On Tue, May 05, 2015 at 06:48:07AM -0700, Bryan O'Donoghue wrote:
> >>+ */
> >>+static int esram_dbgfs_state_show(struct seq_file *s, void *unused)
> >>+{
> >>+	struct esram_dev *edev = &esram_dev;
> >>+	u32 data;
> >>+	u32 reg = (u32)s->private;
> >
> >You really like to waste lines. What's wrong with:
> >
> >     	u32 data, reg = .....
> 
> Hmm, I had feedback when doing the IMR code *not* to do that, so kept that
> pattern for eSRAM. More than happy to rationalize the line-count here.
> 

This had come up in the IMR review, and several times in my other reviews, where
there was a strong preference for one variable per line.

I want to remain consistent with our lead maintainers, and I want to be able to
refer to something as the definitive source.

CodingStyle only has this to say in Chapter 8: Commenting:

  It's also important to comment data, whether they are basic types or derived
  types.  To this end, use just one data declaration per line (no commas for
  multiple data declarations).  This leaves you room for a small comment on each
  item, explaining its use.

Perhaps this ONLY applies if comments are used, but I didn't read it that way.

Personally, I prefer the one per line as it's the easiest to enforce and aligns
with the "decreasing line length" for declarations that Thomas has dinged me for
in the past.

Most importantly though, I just want to be consistent with how I review code so
I'm not guilty of telling Bryan one thing only for him to get dinged for
following it later (again).

Sorry about that Bryan :-)

-- 
Darren Hart
Intel Open Source Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/