Re: [Outreachy kernel] [PATCH] staging: vt6655: Rename `by_preamble_type` parameter

[Karolina Drobnik <karolinadrobnik@gmail.com>]

On Wed, 2021-10-20 at 14:59 +0200, Julia Lawall wrote:
> On Wed, 20 Oct 2021, Karolina Drobnik wrote:
> 
> > On Wed, 2021-10-20 at 10:54 +0200, Julia Lawall wrote:
> > > On Wed, 20 Oct 2021, Karolina Drobnik wrote:
> > > 
> > > > Drop `by` prefix in the first parameter of `bb_get_frame_time`
> > > > function.
> > > > As the original argument, `byPreambleType`, was renamed to
> > > > `preamble_type`,
> > > > the parameter referring to it is now renamed to match the new
> > > > naming
> > > > convention.
> > > > Update `bb_get_frame_time` comment to reflect that change.
> > > > 
> > > > This patch is a follow-up work to this commit:
> > > >     Commit 548b6d7ebfa4 ("staging: vt6655: Rename
> > > > byPreambleType
> > > > field")
> > > 
> > > This is not going to be practical.  If the previous patch is
> > > accepted, then this it not needed.
> > 
> > This change was there before but Greg told me to do only one
> > logical
> > change per patch (which was a struct member rename), so I reverted
> > it.
> > I believe this is needed because this parameter still uses
> > Hungarian
> > notation, which is against the LK coding style. Also, it makes
> > sense to
> > update the name given my previous change.
> 
> Sorry, I think I was not clear.  It's not practical to explain
> constraints
> on other patches in the log message.

Oh, I see. I thought about this log message as "why" but now, when I
come think of it, just saying it's about the Hungarian notation should
be enough. I'll correct the log message, thank you.

> 
> The important thing is
> that if you want to make two different changes on the same file,
> either
> the first one has to be accepted before you submit the second one, or
> they
> have to be in a series.

I see, thanks for clarification.

> > > 
> > I can add this in but will that still count as a one logical
> > change?
> 
> No.  It's a different change.  It's just a small whitespace issue,
> but
> it's not triggered by the other changes you have made.

Ok, I'll submit a separate patch for it later on.


Thanks,
Karolina