lkml.org 
[lkml]   [2015]   [May]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/2] x86/quark: Add Quark embedded SRAM support
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


\
 
 \ /
  Last update: 2015-05-05 22:21    [W:0.041 / U:0.384 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site