Re: [Outreachy kernel] [PATCH v3 2/9] staging: iio: adc: properly indent to match open paranthesis

[Sudip Mukherjee <sudipm.mukherjee@gmail.com>]

On Tue, Oct 13, 2015 at 01:05:58PM +0300, Ioana Ciornei wrote:
> On Tue, Oct 13, 2015 at 12:56 PM, Julia Lawall <julia.lawall@lip6.fr> wrote:
> >
> >
> > On Tue, 13 Oct 2015, Ioana Ciornei wrote:
> >
> >> On Mon, Oct 12, 2015 at 6:14 PM, Daniel Baluta <daniel.baluta@gmail.com> wrote:
> >> >
> >> > On Fri, Oct 9, 2015 at 10:51 PM, Ioana Ciornei <ciorneiioana@gmail.com> wrote:
> >> > > Indent parameters and arguments passed to function calls to match
> >> > > open paranthesis
> >> > >
> >> > > Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
> >> > > ---
> >> > > Changes in v2:
> >> > >     - nothing
> >> > > Changes in v3:
> >> > >     - rework changes in order to avoid introducing new checkpatch warnings
> >> > >
> >> >
> >> > <snip>
> >> >
> >> > > @@ -565,10 +565,11 @@ static int ad7280_attr_init(struct ad7280_state *st)
> >> > >                         st->iio_attr[cnt].dev_attr.store =
> >> > >                                 ad7280_store_balance_sw;
> >> > >                         st->iio_attr[cnt].dev_attr.attr.name =
> >> > > -                               kasprintf(GFP_KERNEL,
> >> >
> >> > This line looks good to me as it is. Is there any particular reason
> >> > for which you we want GFP_KERNEL to be moved on the next line?
> >> >
> >>
> >> I chose to move kasprintf on the next line and all the others
> >> arguments also in order to improve readability.
> >> I had to choose between leaving the kasprintf where it was and
> >> properly align the arguments to match the open bracket but
> >> cross the 80 char limit or moving the kasprintf call on the next line
> >> so the arguments are somewhat distinguishable.
> >>
> >> Was this a bad call?
> >
> > It's not really normal to see the first argument on a different line.
> > Do the other arguments not fit if they are lines up with the right side of
> > the (?  Perhaps it is possible to break up the longer arguments more so
> > that they fit better?
> >
> 
> The first argument fits, the problem is with the other ones.
> Well, now that I think of... yes there is a solution to this.
> If use a variable instead of 'dev * AD7280A_CELLS_PER_DEV + ch' all
> the problems would be solved.
> 
> Is this a proper way to handle it?

Today morning Greg said in a review of one patch "No need to create a
whole new variable when you only use it once."

reference at: https://lkml.org/lkml/2015/10/13/27

regards
sudip