Re: [Outreachy kernel] [PATCH] staging: rtlwifi: Remove unnecessary conditions

From: Maya Nakamura <m.maya.nakamura@gmail.com>
To: Sasha Levin <sashal@kernel.org>
Cc: Julia Lawall <julia.lawall@lip6.fr>,
	gregkh@linuxfoundation.org, outreachy-kernel@googlegroups.com
Subject: Re: [Outreachy kernel] [PATCH] staging: rtlwifi: Remove unnecessary conditions
Date: Tue, 30 Oct 2018 22:01:21 -0700	[thread overview]
Message-ID: <20181031050120.GA3667@k-vBox> (raw)
In-Reply-To: <20181031003131.GF194472@sasha-vm>

On Tue, Oct 30, 2018 at 08:31:31PM -0400, Sasha Levin wrote:
> On Tue, Oct 30, 2018 at 05:06:15PM -0700, Maya Nakamura wrote:
> > On Tue, Oct 30, 2018 at 08:46:21AM +0100, Julia Lawall wrote:
> > > On Mon, 29 Oct 2018, Maya Nakamura wrote:
> > > > @@ -168,9 +159,7 @@ void phydm_init_debug_setting(struct phy_dm_struct *dm)
> > > >  	dm->debug_level = ODM_DBG_TRACE;
> > > >
> > > >  	dm->fw_debug_components = 0;
> > > > -	dm->debug_components =
> > > > -
> > > > -		0;
> > > > +	dm->debug_components = 0;
> > > 
> > > 
> > > Does this belong here?  I don't see an if nearby.  You should make a
> > > series if you want to do two different things on the same file.
> > > 
> > > julia
> > 
> > Thank you, Julia, for reviewing my submission! I will separate different
> > types of fixes into their own patches and resend these.
> 
> Hi Maya,
> 
> I'm guessing that while you were working on your original plan to remove
> unnecessary conditions you saw the weird looking piece of code above and
> decided to fix it as well. As a result, while you dealt nicely with this
> cleanup, you ended up with unrelated changes in the same patch.
> 
> An easy way to work around it is the '-p' flag to 'git add'. This flag
> lets you interactively select hunks to stage in git; you don't have to
> 'git add' and entire file, but can just add parts of it.
> 
> This way, you stage only the relevant hunks and 'git commit' them when
> you're ready. Rinse and repeat for every set of hunks that represents a
> single logical change.
> 
> --
> Thanks,
> Sasha

Thank you for teaching me, Sasha!

You read my mind :-) I included changes that I had not targeted because
I found one and Julia told me about the other, and I thought that it
would be clear if I wrote about them as part of the commit message.
Sorry, lazy me.

I have never used that special flag. That is fantastic!

I will keep in mind: only a single logical type of change in a patch.

Maya