Checks for recipients

Checks for recipients

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 220 bytes --]

Hi,

It would be nice for scripting if b4 had a check similar to the
needs-editing flag in show-info which would allow scripts wrapping it to
determine if there are any recipients specified for the series.

Thanks,
Mark

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Checks for recipients

[Konstantin Ryabitsev]

On Mon, Apr 08, 2024 at 07:48:46PM +0100, Mark Brown wrote:
> It would be nice for scripting if b4 had a check similar to the
> needs-editing flag in show-info which would allow scripts wrapping it to
> determine if there are any recipients specified for the series.

Yes, I'm working on integrating various checks into a single `b4 prep --check`
command:

- check if series cover needs editing
- check if series needs pre-send checks (checkpatch)
- check if dependencies are valid (current --check-deps)
- check if --auto-to-cc has been run
- etc, etc

It's going to take a few iterations before this works reliably, but we'll get
there.

-K

Re: Checks for recipients

[Konstantin Ryabitsev]

On Mon, 8 Apr 2024 at 14:48, Mark Brown <broonie@kernel.org> wrote:
> It would be nice for scripting if b4 had a check similar to the
> needs-editing flag in show-info which would allow scripts wrapping it to
> determine if there are any recipients specified for the series.

Mark:

I added some stuff today that introduces "pre-flight checks." I'm not
sure if it's quite what you were looking for, but --show-info will now
give you the following data:

- if prep --check needs to be (re-)run
- if prep --auto-to-cc needs to be (re-)run

It doesn't try to be very clever -- basically, if the tip-commit of
your series changes, we assume that you'll want to rerun the
prep-check and prep-auto-to-cc, -- but it's effective. I just hope we
are striking the right balance between "useful" and "annoying." :)

Can you give it a try and let me know what you think?

-K

Re: Checks for recipients

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 1554 bytes --]

On Fri, Apr 19, 2024 at 09:37:46PM -0400, Konstantin Ryabitsev wrote:
> On Mon, 8 Apr 2024 at 14:48, Mark Brown <broonie@kernel.org> wrote:

> > It would be nice for scripting if b4 had a check similar to the
> > needs-editing flag in show-info which would allow scripts wrapping it to
> > determine if there are any recipients specified for the series.

> I added some stuff today that introduces "pre-flight checks." I'm not
> sure if it's quite what you were looking for, but --show-info will now
> give you the following data:

> - if prep --check needs to be (re-)run

I can see this being useful but I am not personally going want to
incorporate this into my scripting as checkpatch is just far too prone
to false positives, especially with anything that isn't a simple C file.

> - if prep --auto-to-cc needs to be (re-)run

I similarly wouldn't want to to do this as there are cases where it
pulls in too many people (mainly cases where you only want to send
things to submaintainers rather than including everyone that gets pulled
in automatically).

> It doesn't try to be very clever -- basically, if the tip-commit of
> your series changes, we assume that you'll want to rerun the
> prep-check and prep-auto-to-cc, -- but it's effective. I just hope we
> are striking the right balance between "useful" and "annoying." :)

> Can you give it a try and let me know what you think?

I think for the issue I was reporting above just showing the recipieints
in show-info would be enough, all I'm looking to check for is that the
list isn't empty.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Checks for recipients

[Konstantin Ryabitsev]

On Sat, Apr 20, 2024 at 10:23:19PM +0900, Mark Brown wrote:
> > I added some stuff today that introduces "pre-flight checks." I'm 
> > not
> > sure if it's quite what you were looking for, but --show-info will now
> > give you the following data:
> 
> > - if prep --check needs to be (re-)run
> 
> I can see this being useful but I am not personally going want to
> incorporate this into my scripting as checkpatch is just far too prone
> to false positives, especially with anything that isn't a simple C file.
> 
> > - if prep --auto-to-cc needs to be (re-)run
> 
> I similarly wouldn't want to to do this as there are cases where it
> pulls in too many people (mainly cases where you only want to send
> things to submaintainers rather than including everyone that gets pulled
> in automatically).
> 
> > It doesn't try to be very clever -- basically, if the tip-commit of
> > your series changes, we assume that you'll want to rerun the
> > prep-check and prep-auto-to-cc, -- but it's effective. I just hope we
> > are striking the right balance between "useful" and "annoying." :)
> 
> > Can you give it a try and let me know what you think?
> 
> I think for the issue I was reporting above just showing the recipieints
> in show-info would be enough, all I'm looking to check for is that the
> list isn't empty.

Thanks for your feedback. I'll make these two changes next:

- add a check/report for "needs-recipients" that will be separate from 
  "needs-auto-to-cc"
- add a way to turn off pre-flight checks via b4.prep-pre-flight-checks 
  = off

-K

Re: Checks for recipients

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 513 bytes --]

On Sat, Apr 20, 2024 at 12:29:48PM -0400, Konstantin Ryabitsev wrote:

> Thanks for your feedback. I'll make these two changes next:

> - add a check/report for "needs-recipients" that will be separate from 
>   "needs-auto-to-cc"
> - add a way to turn off pre-flight checks via b4.prep-pre-flight-checks 
>   = off

Thanks - that sounds good.  Perhaps for the pre-flight checks list a
pass/block list mechanism might be good so that people can pick which
checks work well for them in their workflow?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Checks for recipients

[Konstantin Ryabitsev]

On Sun, Apr 21, 2024 at 09:47:39AM +0900, Mark Brown wrote:
> > Thanks for your feedback. I'll make these two changes next:
> 
> > - add a check/report for "needs-recipients" that will be separate from 
> >   "needs-auto-to-cc"
> > - add a way to turn off pre-flight checks via b4.prep-pre-flight-checks 
> >   = off
> 
> Thanks - that sounds good.  Perhaps for the pre-flight checks list a
> pass/block list mechanism might be good so that people can pick which
> checks work well for them in their workflow?

Ok, the 'needs-recipients' check is in now.

I'll work on adding a way to turn off pre-flight checks via:

  b4.prep-pre-flight-checks: disable-all
  b4.prep-pre-flight-checks: disable-needs-auto-to-cc, disable-needs-checking

This should allow turning off either all checks, or just specific ones.

-K

Re: Checks for recipients

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 939 bytes --]

Hi Konstantin,

With current git b4 (621fa62) I'm getting tracebacks doing

   b4 update --trailers-from ${MSGID} --since ${COMMIT}

(I'm using ZktD50C5twF1EuKu@fedora or ZktCpcGZdgHWuN_L@fedora as msgid
here, ${COMMIT} being what I applied those two to):

Calculating patch-ids from commits, this may take a moment...
Traceback (most recent call last):
  File "/home/broonie/git/b4/src/b4/command.py", line 428, in <module>
    cmd()
  File "/home/broonie/git/b4/src/b4/command.py", line 410, in cmd
    cmdargs.func(cmdargs)
  File "/home/broonie/git/b4/src/b4/command.py", line 85, in cmd_trailers
    b4.ez.cmd_trailers(cmdargs)
  File "/home/broonie/git/b4/src/b4/ez.py", line 2786, in cmd_trailers
    update_trailers(cmdargs)
  File "/home/broonie/git/b4/src/b4/ez.py", line 1107, in update_trailers
    if lmsg.followup_trailers:
       ^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'NoneType' object has no attribute 'followup_trailers'

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: Checks for recipients

[Konstantin Ryabitsev]

On Mon, May 20, 2024 at 02:45:30PM GMT, Mark Brown wrote:
> Hi Konstantin,
> 
> With current git b4 (621fa62) I'm getting tracebacks doing
> 
>    b4 update --trailers-from ${MSGID} --since ${COMMIT}
> 
> (I'm using ZktD50C5twF1EuKu@fedora or ZktCpcGZdgHWuN_L@fedora as msgid
> here, ${COMMIT} being what I applied those two to):
> 
> Calculating patch-ids from commits, this may take a moment...
> Traceback (most recent call last):
>   File "/home/broonie/git/b4/src/b4/command.py", line 428, in <module>
>     cmd()
>   File "/home/broonie/git/b4/src/b4/command.py", line 410, in cmd
>     cmdargs.func(cmdargs)
>   File "/home/broonie/git/b4/src/b4/command.py", line 85, in cmd_trailers
>     b4.ez.cmd_trailers(cmdargs)
>   File "/home/broonie/git/b4/src/b4/ez.py", line 2786, in cmd_trailers
>     update_trailers(cmdargs)
>   File "/home/broonie/git/b4/src/b4/ez.py", line 1107, in update_trailers
>     if lmsg.followup_trailers:
>        ^^^^^^^^^^^^^^^^^^^^^^
> AttributeError: 'NoneType' object has no attribute 'followup_trailers'

bugbot assign to me

Re: Checks for recipients

[Kernel.org Bugbot]

Hello:

This conversation is now tracked by Kernel.org Bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=218878

There is no need to do anything else, just keep talking.
-- 
Deet-doot-dot, I am a bot.
Kernel.org Bugzilla (peebz 0.1)

Re: Checks for recipients

[Kernel.org Bugbot]

Konstantin Ryabitsev writes in commit 3bb13877d372002e91c463d808d6608de341195e:

ez: don't crash on trailers -u and partial series

When we can only get a partial series from a thread specified via the
cmdline, do not crash when retrieving followup-trailers for a missing
patch.

Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=218878
Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>

(via https://git.kernel.org/pub/scm/utils/b4/b4.git/commit/?id=3bb13877d372)
-- 
Deet-doot-dot, I am a bot.
Kernel.org Bugzilla (peebz 0.1)