lkml.org 
[lkml]   [2020]   [Mar]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [Outreachy kernel] [PATCH] Staging: rtl8188eu: Add space around operators
From
Date
On Mon, 2020-03-09 at 08:35 +0100, Julia Lawall wrote:
> On Mon, 9 Mar 2020, Shreeya Patel wrote:
>
> > On Sun, 2020-03-08 at 16:05 -0700, Joe Perches wrote:
> > > On Mon, 2020-03-09 at 03:30 +0530, Shreeya Patel wrote:
> > > > Add space around operators for improving the code
> > > > readability.
> > >
> > > Hello again Shreeya.
> > >
> >
> > I have some questions here...
> >
> > > The subject isn't really quite appropriate as you
> > > are not doing this space around operator addition
> > > for the entire subsystem.
> > >
> > > IMO, the subject should be:
> > >
> > > [PATCH] staging: rtl8188eu: rtw_mlme: Add spaces around operators
> > >
> > > because you are only performing this change on this
> > > single file.
> > >
> > > If you were to do this for every single file in the
> > > subsystem, you could have many individual patches with
> > > the exact same subject line.
> > >
> > > And it would be good to show in the changelog that you
> > > have compiled the file pre and post patch without object
> > > code change.
> > >
> >
> > I'm not sure how to show this. Do you mean to add the output of
> > "make drivers/staging/rtl8188eu/core" before and after the changes?
>
> You are working on one specific file, maybe foo.c. Compile before
> making changes, which will give you foo.o. Rename that file to
> something
> else. Make your changes and compile again. Do a diff with the
> previously
> compiled file. It should produce nothing, indicating no difference.
>
> If this .o file doesn't change and you only changed this .c file, the
> whole compiled driver won't change either.
>

ok, got it.

> > I also don't understand the meaning of no object code change. If we
> > are
> > making the changes to code and then compiling it using the make
> > command
> > then a new file with .o extension is created and replaced by the
> > previous one isn't it?
> >
> > > Also, it's good to show that git diff -w shows no source
> > > file changes.
> > >
> >
> > And this has to be...
> > git diff -w --shortstat drivers/staging/rtl8188eu/core/
>
> --shortstat does not seem useful. What you hope to see is that it
> produces nothing.
>
Okay.
I will send a V2 with all the changes required.

Btw Joe, I am working against staging-testing tree

> julia
>
> > Am I correct?
> >
> > Thanks
> >
> > > > Reported by checkpatch.pl
> > > >
> > > > Signed-off-by: Shreeya Patel <shreeya.patel23498@gmail.com>
> > > > ---
> > > > drivers/staging/rtl8188eu/core/rtw_mlme.c | 40 +++++++++++--
> > > > ----
> > > > ------
> > > > 1 file changed, 20 insertions(+), 20 deletions(-)
> > >
> > > When I try this using checkpatch --fix-inplace, I get
> > > 21 changes against the latest -next tree.
> > >
> > > What tree are you doing this against?
> > >
> > >
> >
> > --
> > You received this message because you are subscribed to the Google
> > Groups "outreachy-kernel" group.
> > To unsubscribe from this group and stop receiving emails from it,
> > send an email to outreachy-kernel+unsubscribe@googlegroups.com.
> > To view this discussion on the web visit
> > https://groups.google.com/d/msgid/outreachy-kernel/af1a27fb8c5f7efbaf99ce3055cf3801b366d627.camel%40gmail.com
> > .
> >

\
 
 \ /
  Last update: 2020-03-09 14:26    [W:0.057 / U:1.364 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site