Re: [PATCH v2] staging: greybus: remove tabs to fix line length and merge lines

[Deepak R Varma]

On Sat, Mar 11, 2023 at 11:24:20AM +0500, Khadija Kamran wrote:
> On Sat, Mar 11, 2023 at 07:16:19AM +0300, Dan Carpenter wrote:
> > On Sat, Mar 11, 2023 at 03:18:04AM +0500, Khadija Kamran wrote:
> > > In file drivers/staging/greybus/arche-platform.c,
> > > - Length of line 181 exceeds 100 columns, fix by removing tabs from the
> > >   line.
> > > - If condition and spin_unlock_...() call is split into two lines, join
> > > them to form a single line.
> > > 
> > > Signed-off-by: Khadija Kamran <kamrankhadijadj@xxxxxxxxx>
> > > ---
> > > Changes in v2:
> > >  - Change the subject and log message
> > >  - Merge if condition and spin_unlock...() from two lines to one 
> > 
> > Apply your patch and then re-run checkpatch.pl -f on the file.  You will
> > see the problem.
> 
> Hey Dan!
> When I run checkpatch.pl on my file, I can see that my old CHECK no
> longer exists instead a new CHECK is mentioned saying 'Alignment should
> match open parenthesis'. I understand this from your previous email.
> Should I stop working on this file and leave it as is?

Hi Dan,
Not trying to speak for you, so please override my message if this is
inaccurate.

Hi Khadija,
Yes. It is not useful to resolve one warning and introduce another. Tomorrow
someone else is going to try and revert it. So do not make the "remove tab"
change. I still like the merging of the split lines. It appears to improve code
readability. You can send in a v3 with just that merge change and wait for
feedback.

Also, remember to check your change with checkpatch. There is a section about
post-commit hooks on the tutorials page. This will allow you to integrate
checkpatch as part of your git commit step and do the job for you.

And also, always build your change locally on your machine. No new warnings or
errors should arise.

Hope that helps.
Deepak.


> Thank you!
> >
> > regards,
> > dan carpenter
>