[Linux-kernel-mentees] Possible new warning for checkpatch

[Linux-kernel-mentees] Possible new warning for checkpatch

[Dwaipayan Ray]

Hi Lukas,
I was having a talk with Peilin and a possible new idea came up.
It's about lines in the commit message that start with a '#'.

Normally if a patch contains lines starting with '#' in the commit
message, when they are applied they successfully appear in the
git log. But if a maintainer for some reason decides to rebase and
reword the commit message for whatever reason, the # lines are gone.

Peilin had a look at it and he was able to successfully reproduce this
fault.

Now would it make sense if a warning for such lines starting with '#' in
the commit message are emitted by checkpatch itself? I have no idea
what other developers do, so I could be wrong at this point. But I would
like your opinion.

Thanks,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] Possible new warning for checkpatch

[Peilin Ye]

On Mon, Nov 30, 2020 at 08:28:23PM +0530, Dwaipayan Ray wrote:
> Hi Lukas,
> I was having a talk with Peilin and a possible new idea came up.
> It's about lines in the commit message that start with a '#'.
>
> Normally if a patch contains lines starting with '#' in the commit
> message, when they are applied they successfully appear in the
> git log. But if a maintainer for some reason decides to rebase and
> reword the commit message for whatever reason, the # lines are gone.

Thanks for bringing this up Dwaipayan!

Yes, for example, if one included some code examples (e.g. `#define ...`)
in the commit message, then the maintainer applied, and reworded the
commit to add their own Signed-off-by:, then all the # lines are gone.

> Peilin had a look at it and he was able to successfully reproduce this
> fault.
>
> Now would it make sense if a warning for such lines starting with '#' in
> the commit message are emitted by checkpatch itself? I have no idea
> what other developers do, so I could be wrong at this point. But I would
> like your opinion.

I admit it is a beginner's mistake, but I myself learned it in a hard
way (i.e. by having a patch mainline'd then seeing these # lines gone),
so personally I hope this becomes a new feature of checkpatch.

Thank you,
Peilin Ye

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] Possible new warning for checkpatch

[Lukas Bulwahn]

On Mon, Nov 30, 2020 at 4:23 PM Peilin Ye <yepeilin.cs@gmail.com> wrote:
>
> On Mon, Nov 30, 2020 at 08:28:23PM +0530, Dwaipayan Ray wrote:
> > Hi Lukas,
> > I was having a talk with Peilin and a possible new idea came up.
> > It's about lines in the commit message that start with a '#'.
> >
> > Normally if a patch contains lines starting with '#' in the commit
> > message, when they are applied they successfully appear in the
> > git log. But if a maintainer for some reason decides to rebase and
> > reword the commit message for whatever reason, the # lines are gone.
>
> Thanks for bringing this up Dwaipayan!
>
> Yes, for example, if one included some code examples (e.g. `#define ...`)
> in the commit message, then the maintainer applied, and reworded the
> commit to add their own Signed-off-by:, then all the # lines are gone.
>
> > Peilin had a look at it and he was able to successfully reproduce this
> > fault.
> >
> > Now would it make sense if a warning for such lines starting with '#' in
> > the commit message are emitted by checkpatch itself? I have no idea
> > what other developers do, so I could be wrong at this point. But I would
> > like your opinion.
>
> I admit it is a beginner's mistake, but I myself learned it in a hard
> way (i.e. by having a patch mainline'd then seeing these # lines gone),
> so personally I hope this becomes a new feature of checkpatch.
>

I think this makes sense and checkpatch should warn about that;
probably the easy way to fix it is just to indent such lines by a few
spaces, e.g., four spaces.

Lukas

> Thank you,
> Peilin Ye
>
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] Possible new warning for checkpatch

[Peilin Ye]

Hi Lukas,

On Mon, Nov 30, 2020 at 04:51:42PM +0100, Lukas Bulwahn wrote:
> On Mon, Nov 30, 2020 at 4:23 PM Peilin Ye <yepeilin.cs@gmail.com> wrote:
> >
> > On Mon, Nov 30, 2020 at 08:28:23PM +0530, Dwaipayan Ray wrote:
> > > Hi Lukas,
> > > I was having a talk with Peilin and a possible new idea came up.
> > > It's about lines in the commit message that start with a '#'.
> > >
> > > Normally if a patch contains lines starting with '#' in the commit
> > > message, when they are applied they successfully appear in the
> > > git log. But if a maintainer for some reason decides to rebase and
> > > reword the commit message for whatever reason, the # lines are gone.
> >
> > Thanks for bringing this up Dwaipayan!
> >
> > Yes, for example, if one included some code examples (e.g. `#define ...`)
> > in the commit message, then the maintainer applied, and reworded the
> > commit to add their own Signed-off-by:, then all the # lines are gone.
> >
> > > Peilin had a look at it and he was able to successfully reproduce this
> > > fault.
> > >
> > > Now would it make sense if a warning for such lines starting with '#' in
> > > the commit message are emitted by checkpatch itself? I have no idea
> > > what other developers do, so I could be wrong at this point. But I would
> > > like your opinion.
> >
> > I admit it is a beginner's mistake, but I myself learned it in a hard
> > way (i.e. by having a patch mainline'd then seeing these # lines gone),
> > so personally I hope this becomes a new feature of checkpatch.
> 
> I think this makes sense and checkpatch should warn about that;
> probably the easy way to fix it is just to indent such lines by a few
> spaces, e.g., four spaces.

Yes, that prevents them from being dropped accidentally. Thank you,

Peilin Ye

_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees

Re: [Linux-kernel-mentees] Possible new warning for checkpatch

[Dwaipayan Ray]

On Mon, Nov 30, 2020 at 9:43 PM Peilin Ye <yepeilin.cs@gmail.com> wrote:
>
> Hi Lukas,
>
> On Mon, Nov 30, 2020 at 04:51:42PM +0100, Lukas Bulwahn wrote:
> > On Mon, Nov 30, 2020 at 4:23 PM Peilin Ye <yepeilin.cs@gmail.com> wrote:
> > >
> > > On Mon, Nov 30, 2020 at 08:28:23PM +0530, Dwaipayan Ray wrote:
> > > > Hi Lukas,
> > > > I was having a talk with Peilin and a possible new idea came up.
> > > > It's about lines in the commit message that start with a '#'.
> > > >
> > > > Normally if a patch contains lines starting with '#' in the commit
> > > > message, when they are applied they successfully appear in the
> > > > git log. But if a maintainer for some reason decides to rebase and
> > > > reword the commit message for whatever reason, the # lines are gone.
> > >
> > > Thanks for bringing this up Dwaipayan!
> > >
> > > Yes, for example, if one included some code examples (e.g. `#define ...`)
> > > in the commit message, then the maintainer applied, and reworded the
> > > commit to add their own Signed-off-by:, then all the # lines are gone.
> > >
> > > > Peilin had a look at it and he was able to successfully reproduce this
> > > > fault.
> > > >
> > > > Now would it make sense if a warning for such lines starting with '#' in
> > > > the commit message are emitted by checkpatch itself? I have no idea
> > > > what other developers do, so I could be wrong at this point. But I would
> > > > like your opinion.
> > >
> > > I admit it is a beginner's mistake, but I myself learned it in a hard
> > > way (i.e. by having a patch mainline'd then seeing these # lines gone),
> > > so personally I hope this becomes a new feature of checkpatch.
> >
> > I think this makes sense and checkpatch should warn about that;
> > probably the easy way to fix it is just to indent such lines by a few
> > spaces, e.g., four spaces.
>
> Yes, that prevents them from being dropped accidentally. Thank you,
>

Yep, that sounds good to me too. I will send in a patch soon.

Thanks,
Dwaipayan.
_______________________________________________
Linux-kernel-mentees mailing list
Linux-kernel-mentees@lists.linuxfoundation.org
https://lists.linuxfoundation.org/mailman/listinfo/linux-kernel-mentees