Re: b4 review available in master

[Mark Brown <broonie@kernel.org>]

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

On Sat, Feb 28, 2026 at 03:53:05PM +0000, Mark Brown wrote:
> On Fri, Feb 27, 2026 at 02:53:07PM -0500, Konstantin Ryabitsev wrote:

> >   - AI-assisted review using an external agent (Claude Code, Gemini CLI,
> >     OpenAI Codex CLI, GitHub Copilot CLI) — agent findings are private to the
> >     maintainer and can then be selectively incorporated into the review
> >   - Patchwork integration: browse series, view CI check results, track series
> >     directly from Patchwork, and automatically synchronise Patchwork state as
> >     you work
> 
> As requested since it looks like the textual thing I reported might be a
> bit annoying to resolve a couple of bits of feedback just from reading
> the announce:

I've now managed to actually play with this a bit.  It looks really
great thus far, thanks a lot for working on it.  I've brain dumped a
bunch of thoughts below which I appreciate is a bunch of
demanding/moaning but please don't let that take away from the overall
positive first impression.

>  - It'd be good to have a workflow for syncing the reviews between
>    machines (with privacy!), for example I move between my laptop and
>    desktop semi regularly.
>  - An integration with non-patchwork CI would be useful, perhaps this
>    looks a bit like the agent stuff above?  It's "hand off to an
>    external tool and some time later see what the tool/a separate
>    results collection tool had to say about things"?

Expanding on this a bit what I have ATM is effectively two operations
both of which take commit ranges, one to start testing and another which
will either say everything is OK or provide a list of outstanding issues
(one of which might be that it's still waiting for results, that'd
probably be useful to distinguish in UI).

>  - If this works for me I think I'm likely to want an "applied but not
>    taken" state, I CI the actual commits I end up merging and do end up
>    looking back at the results sometimes.

Having now actually used this I now see that the commits are actually
being applied as part of the storage for the review so a lot of this is
already happening (though without signoffs), it seems like this is quite
close to what I'm looking for with having the actual commits that could
be merged available with Link: and signoff.  I think I'm just suggesting
moving the bit that adds those two from the apply part of the flow to
the import part of the flow.

I can see it might worrying people to have those commits there, but I'd
definitely use this and I think the merge application strategy might
actually be relying on this?

>  - My existing tooling has an "I'll apply this at rcX" state which I use
>    moderately heavily when I'm happy but want to allow time for other
>    people to review.  Other units of time are available, I'm mostly
>    using the rcs as a proxy for that (and it's very useful for "apply
>    this after the merge window closes).

Other things that came up:

 - The initial import of the series and actions->rebase appear to not
   allow the target branch to be specified, but taking does.  It'd be
   good if this were selectable consistently, and I suspect if a base
   has been explicitly specified for a branch we'd want to default to
   rebasing against that.

 - Some ability to either configure a list of branches to apply to or 
   just a recently used list of branches would be handy.

 - It'd be good to have the ability to sort/filter by activity in the
   series view ("What serieses have active discussion and should have
   attention paid?", "Where's discussion died down and I should think
   about applying?").

 - Adding signoffs and Link: tags to applied serieses doesn't seem to
   work for me with the merge strategy.  I think this is because it's
   doing what I suggested above and reusing the commits that were being
   used for review?  Linear strategy does the right thing.

 - It feels like the reply and comment functionality isn't as joined up
   as it could be.  Adding a comment doesn't block replies, but if
   you've replied it's not possible to comment further.  I think I was
   expecting that replies would be stored as a series of comments
   injected at the point where the quoted text was interrupted and I'd
   be free to switch between the two so long as I didn't edit the quoted
   text.  As it is you can comment but once you've been in reply mode it
   is no longer possible to use the comment functionality.  I do realise
   that there is some fun with detecting that this is the case.

 - At the minute cherry picking of individual patches can happen only at
   merge time, it'd be useful to be able to do this during review.  For
   example, with multi-subsystem serieses (especially large cleanup
   serieses that really shouldn't have been a single series in the first
   place) it can be easier to discard irrelevant bits so you can
   actually find the bit you want to look at.

 - I edited a dummy reply to be To: another of my email addresses but
   when I tried sending I got a mail To: broonie@kernel.org, Cc the new
   address.  Copying my main address is good for my mail archives, but
   I'd have expected the To: to be what was configured.  The metadata
   looks good in the UI if I hit 'T' to edit To/Cc but it's wrong in
   both the TUI and what actually gets sent.  Quite possibly user error,
   I didn't look too hard.

 - If you use the comment feature and don't go into reply mode then the
   entire diff chunk up to the line commented on will be quoted.  This
   can blow up badly if the hunk is very big, for example when adding a
   new file.  It's a sensible default but some mitigation might be in
   order, for example warning if the quoted percentage is exessively
   high and suggesting editing in reply mode.

 - I can't see a way to add notes to trailers, for example a comment
   about where/how something was tested.

 - Any review on a patch causes a tick to be shown against it in the UI,
   for larger or complicated patches it'd be good to have more control
   of this so you could see in the summary if you're all done with the
   patch.  The tick appears even if you added but did not send comments
   or a reply which feels like a landmine.

 - Especially given that there's not a key collision it feels a bit odd
   that at least the send part of the reply functionality requires
   switching to email mode.

 - The UI for followup comments feels like it needs fleshing out:

   - It feels like it should be more obvious from the initial series
     view that followups exist (especially if they were previously
     fetched) in either the list of serieses or the per-series view.

   - Some way of tracking status for followups would be good (eg, read,
     needs addressing, needs reply).

   - The UI for followups shows each person's comments as a single
     summary which doesn't really give a picture of how active the 
     discussion is or anything (mutt's thread view is great here!).

   - It doesn't seem to be possible to reply to a followup?  The feels
     like it's getting into "switch to your mail client" territory
     though.

   - It'd be good to be able to see message IDs for followups to make it
     easier to find them in your mail client.

 - Does the thank you note stuff integrate with b4 ty or have an
   equivalent thereof?  I purposely generate the thank you notes by
   asking b4 to check what's in my public branches so people don't get
   confused by seeing the mail before I did a git push (that used to
   happen often enough to be noticable).  It doesn't seem to AFAICT.

 - pgp signing mails would be nice but very, very low priority.

Like I say thanks for working on this.

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