* Re: b4 review available in master
2026-02-27 19:53 b4 review available in master Konstantin Ryabitsev
@ 2026-02-28 15:12 ` Mark Brown
2026-02-28 15:47 ` Konstantin Ryabitsev
2026-02-28 15:53 ` Mark Brown
` (8 subsequent siblings)
9 siblings, 1 reply; 69+ messages in thread
From: Mark Brown @ 2026-02-28 15:12 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 730 bytes --]
On Fri, Feb 27, 2026 at 02:53:07PM -0500, Konstantin Ryabitsev wrote:
> The only new dependency introduced is python-textual. I say this with a bit of
> tongue-in-cheek, because textual pulls in a ton of other dependencies, but
> it's usually already installed on many modern distros anyway.
On Debian stable:
class ReviewApp(App[None]):
...<1351 lines>...
self.push_screen(HelpScreen(_review_help_lines(has_agent=has_agent, has_check=has_check)))
File "/home/broonie/git/b4/src/b4/review_tui/_review_app.py", line 138, in ReviewApp
_GRP_REVIEW = Binding.Group('Review')
^^^^^^^^^^^^^
AttributeError: type object 'Binding' has no attribute 'Group'
where python3-textual is 2.1.2-1.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-28 15:12 ` Mark Brown
@ 2026-02-28 15:47 ` Konstantin Ryabitsev
2026-02-28 16:00 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-02-28 15:47 UTC (permalink / raw)
To: Mark Brown; +Cc: users, tools
On Sat, Feb 28, 2026 at 03:12:28PM +0000, Mark Brown wrote:
> > The only new dependency introduced is python-textual. I say this with a bit of
> > tongue-in-cheek, because textual pulls in a ton of other dependencies, but
> > it's usually already installed on many modern distros anyway.
>
> On Debian stable:
>
> class ReviewApp(App[None]):
> ...<1351 lines>...
> self.push_screen(HelpScreen(_review_help_lines(has_agent=has_agent, has_check=has_check)))
> File "/home/broonie/git/b4/src/b4/review_tui/_review_app.py", line 138, in ReviewApp
> _GRP_REVIEW = Binding.Group('Review')
> ^^^^^^^^^^^^^
> AttributeError: type object 'Binding' has no attribute 'Group'
>
> where python3-textual is 2.1.2-1.
You're right, looks like the stuff I put in to make the status bar look nicer
pulls in textual >= 6.0, released in Aug 2025. I pushed the fix that sets the
requirement properly, but I'll also check if I can make it work with what's in
debian.
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-28 15:47 ` Konstantin Ryabitsev
@ 2026-02-28 16:00 ` Konstantin Ryabitsev
2026-02-28 18:12 ` Mark Brown
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-02-28 16:00 UTC (permalink / raw)
To: Mark Brown; +Cc: users, tools
On Sat, Feb 28, 2026 at 10:47:21AM -0500, Konstantin Ryabitsev wrote:
> > On Debian stable:
> >
> > class ReviewApp(App[None]):
> > ...<1351 lines>...
> > self.push_screen(HelpScreen(_review_help_lines(has_agent=has_agent, has_check=has_check)))
> > File "/home/broonie/git/b4/src/b4/review_tui/_review_app.py", line 138, in ReviewApp
> > _GRP_REVIEW = Binding.Group('Review')
> > ^^^^^^^^^^^^^
> > AttributeError: type object 'Binding' has no attribute 'Group'
> >
> > where python3-textual is 2.1.2-1.
>
> You're right, looks like the stuff I put in to make the status bar look nicer
> pulls in textual >= 6.0, released in Aug 2025. I pushed the fix that sets the
> requirement properly, but I'll also check if I can make it work with what's in
> debian.
Actually, this was a straightforward rewrite -- the latest master should work
for you without a custom venv and the textual package available in Debian.
I've also confirmed that it works on Fedora-43 with python-textual from system
libs.
Thanks for the report!
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-27 19:53 b4 review available in master Konstantin Ryabitsev
2026-02-28 15:12 ` Mark Brown
@ 2026-02-28 15:53 ` Mark Brown
2026-02-28 21:11 ` Mark Brown
2026-02-28 16:36 ` Conor Dooley
` (7 subsequent siblings)
9 siblings, 1 reply; 69+ messages in thread
From: Mark Brown @ 2026-02-28 15:53 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 1589 bytes --]
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:
- 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"?
- 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.
- 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).
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-28 15:53 ` Mark Brown
@ 2026-02-28 21:11 ` Mark Brown
2026-03-03 5:14 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Mark Brown @ 2026-02-28 21:11 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- 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 --]
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-28 21:11 ` Mark Brown
@ 2026-03-03 5:14 ` Konstantin Ryabitsev
2026-03-03 12:42 ` Mark Brown
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-03 5:14 UTC (permalink / raw)
To: Mark Brown; +Cc: users, tools
On Sat, Feb 28, 2026 at 09:11:30PM +0000, Mark Brown wrote:
> 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.
Thanks for trying it out!
> > - 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.
This was already kinda working, but it should work better now. "With privacy"
is the complicated part, because we really-really don't want to deal with
encryption and keys and all the stuff that comes with that. I recommend just
having a private remote where you can push your review branches. It's not very
well-documented, but you have a private space in
git@gitolite.kernel.org:scratch/broonie/ -- don't let the name "scratch" spook
you, it's just not replicated to git.kernel.org, that's all.
> > - 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).
I'll ponder how to best do it. It's possible this may be best accomplished
with a wrapper, but I've not given this much thought yet.
> > - 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.
Yes, I'm thinking a "triaging" stage if you have a review-triaging-branch
defined. This will be the "applied but not taken" state. I'll ponder this
next.
> 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.
We can now define review-target-branch, but I don't think it's wired into the
"rebase" flow yet, just in the "take" flow. I'll make a note to check both.
> - Some ability to either configure a list of branches to apply to or
> just a recently used list of branches would be handy.
The b4.review-target-branch is now settable. I think if you want to apply to
different branches, the sanest approach is to use worktrees and set different
git configs for them -- we'll pick whatever git says is the most relevant
b4.review-target-branch setting for that worktree.
> - 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?").
The latest review UI does sort by activity. The series most recently updated
will be at the top. You can run a full update with "u" to pull in the latest
from lore.
> - 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.
This is "as designed" right now -- the Signed-off-by and Link trailers are
applied to the merge commit and leave the individual commits untouched. It's a
controversial take, I know. :)
Would you prefer that we always applied Signed-off-by: and Link: to each
individual commit even if we're doing a merge?
> - 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.
There is more logic in the review UI now regarding comments and replies, but
more work is needed to make it a more natural flow. In general, the idea is
that you'll do inline comments first (c) and then tailor your reply with "r".
However, both parts need polish.
> - 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'll push back on this a little simply because there's a lot that is keyed off
the number of patches in the series. If we cherry-pick before we even review,
this makes a lot of other functionality fragile. However, there is code now to
mark patches as "skip" (with x) -- this should let you completely ignore them.
When we "take" a series with ignored patches we will automatically default to
the cherry-pick strategy and pre-unselect the ignored patches. Would that come
kinda close to what you want?
> - 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.
I don't think I fixed this yet, but there's been churn in that code today. See
if it's still doing that.
> - 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.
This is now handled with better defaults, but more testing is needed!
> - I can't see a way to add notes to trailers, for example a comment
> about where/how something was tested.
You can't sorry. The "t" screen is really a fire-and-done kinda approach with
no nuance. If you want to add a note to a trailer, the best way is to do "t"
and then "r" to tweak the reply, but I just realized that we don't populate
that with the trailers. I'll see what is the best course of action there.
> - 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.
There is now a special "done" mode that lets you indicate that you're done
reviewing a patch as opposed to just added comments. We won't send anything if
you have any draft patches.
> - 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.
Because I'm proud of that mode! :) But, seriously, I mostly wanted to give
maintainers an overview "hey, this is what will be sent, check it before you
send it." I may have been burned by "b4 ty" being too happy to be totally
wrong by default.
> - 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.
We now show this on the tracking ui and store that in the repo. The "f" is now
more of a toggle, really.
> - Some way of tracking status for followups would be good (eg, read,
> needs addressing, needs reply).
Still TBD.
> - 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 does now nest them a bit, plus will show total vs. new follow-ups in the
tracking ui, but testing is required.
> - 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.
I didn't want to do it initially, but I do realize that it's a huge pain to
switch to an email client if you just want to send a quick follow-up. There is
now code that will let you send a follow-up, but it's a bit rough around the
edges. You have to click on the little backarrow at the top of the follow-up
frame. I'm still poking around and that and should make it more obvious, but
the core functionality is now there.
> - It'd be good to be able to see message IDs for followups to make it
> easier to find them in your mail client.
It does show them now.
> - Does the thank you note stuff integrate with b4 ty or have an
> equivalent thereof?
It's an equivalent of it and will only do "b4 review" taken series. On the
upside, it completely eliminates guesswork and will actually record sha's when
you run "take".
> 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.
Right, I'll see if we can make this check semi-automatic.
> - pgp signing mails would be nice but very, very low priority.
I don't think we'll get there, but we can patatt-sign everything (yes, even
the follow-ups). This will at least get you nice checkmarks in "b4 shazam" for
your trailers.
> Like I say thanks for working on this.
Thanks for being the willing guinea pig! :)
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-03-03 5:14 ` Konstantin Ryabitsev
@ 2026-03-03 12:42 ` Mark Brown
2026-03-03 18:21 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Mark Brown @ 2026-03-03 12:42 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 7578 bytes --]
On Tue, Mar 03, 2026 at 12:14:08AM -0500, Konstantin Ryabitsev wrote:
> On Sat, Feb 28, 2026 at 09:11:30PM +0000, Mark Brown wrote:
> > > - 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.
> This was already kinda working, but it should work better now. "With privacy"
> is the complicated part, because we really-really don't want to deal with
> encryption and keys and all the stuff that comes with that. I recommend just
> having a private remote where you can push your review branches. It's not very
> well-documented, but you have a private space in
> git@gitolite.kernel.org:scratch/broonie/ -- don't let the name "scratch" spook
> you, it's just not replicated to git.kernel.org, that's all.
Yeah, pushing branches isn't a concern - it was more the database bit.
> > > - 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).
> I'll ponder how to best do it. It's possible this may be best accomplished
> with a wrapper, but I've not given this much thought yet.
I'd expect to have to write a script - I was thinking that the agent
stuff you've got already is basically "shell out to a tool and have it
output results in a given format which we pull in", that seemed like an
OK shape for something like this.
> > - 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.
> We can now define review-target-branch, but I don't think it's wired into the
> "rebase" flow yet, just in the "take" flow. I'll make a note to check both.
I'd expect many people have at least fixes and -next branches in the
same git repo (I have that, times four for each subsystem I have) so
it'd need to be runtime selectable which branch to use.
> > - Some ability to either configure a list of branches to apply to or
> > just a recently used list of branches would be handy.
>
> The b4.review-target-branch is now settable. I think if you want to apply to
> different branches, the sanest approach is to use worktrees and set different
> git configs for them -- we'll pick whatever git says is the most relevant
> b4.review-target-branch setting for that worktree.
> > - 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?").
> The latest review UI does sort by activity. The series most recently updated
> will be at the top. You can run a full update with "u" to pull in the latest
> from lore.
Ah, OK - it wasn't clear if that was the date things were sent or the
date there was most reently activity.
> > - 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.
> This is "as designed" right now -- the Signed-off-by and Link trailers are
> applied to the merge commit and leave the individual commits untouched. It's a
> controversial take, I know. :)
> Would you prefer that we always applied Signed-off-by: and Link: to each
> individual commit even if we're doing a merge?
*Especially* if we're doing a merge, I think the general expectation is
that there will be a signoff from the person who committed the patch -
in -next we check every commit for both author and committer signoffs.
If we cherry pick and they get applied then that'd work out OK, but if
we merge and the unsigned commits end up in -next that'd be an issue.
Missing signoffs will also mess up with flows like cherry picking
patches back into stable, the merge would be long gone by the time the
commit shows up in stable.
> > - 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.
> There is more logic in the review UI now regarding comments and replies, but
> more work is needed to make it a more natural flow. In general, the idea is
> that you'll do inline comments first (c) and then tailor your reply with "r".
> However, both parts need polish.
FWIW I find I moderately often come up with a top level comment part way
through the reply and go back to the top of the reply to add it.
> > - 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'll push back on this a little simply because there's a lot that is keyed off
> the number of patches in the series. If we cherry-pick before we even review,
> this makes a lot of other functionality fragile. However, there is code now to
> mark patches as "skip" (with x) -- this should let you completely ignore them.
> When we "take" a series with ignored patches we will automatically default to
> the cherry-pick strategy and pre-unselect the ignored patches. Would that come
> kinda close to what you want?
Possibly? I'd need to try it. Some of these serieses are quite large
so even finding the relevant mails can be annoying.
> > - 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.
> I don't think I fixed this yet, but there's been churn in that code today. See
> if it's still doing that.
Yeah, still seems to be present.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-03-03 12:42 ` Mark Brown
@ 2026-03-03 18:21 ` Konstantin Ryabitsev
2026-03-12 17:21 ` Alexandre Belloni
2026-03-12 17:35 ` Mark Brown
0 siblings, 2 replies; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-03 18:21 UTC (permalink / raw)
To: Mark Brown; +Cc: users, tools
On Tue, Mar 03, 2026 at 12:42:26PM +0000, Mark Brown wrote:
> > This was already kinda working, but it should work better now. "With privacy"
> > is the complicated part, because we really-really don't want to deal with
> > encryption and keys and all the stuff that comes with that. I recommend just
> > having a private remote where you can push your review branches. It's not very
> > well-documented, but you have a private space in
> > git@gitolite.kernel.org:scratch/broonie/ -- don't let the name "scratch" spook
> > you, it's just not replicated to git.kernel.org, that's all.
>
> Yeah, pushing branches isn't a concern - it was more the database bit.
The database is mostly there as a caching layer. We do a "rescan" on each
application start and will update the database with whatever we find in actual
review branches. So, you should be able to push and pull between systems and
have it reasonably synchronized (bugs excepted). It's documented here:
https://b4.docs.kernel.org/en/latest/maintainer/review.html#setting-up-a-sync-remote
> > We can now define review-target-branch, but I don't think it's wired into the
> > "rebase" flow yet, just in the "take" flow. I'll make a note to check both.
>
> I'd expect many people have at least fixes and -next branches in the
> same git repo (I have that, times four for each subsystem I have) so
> it'd need to be runtime selectable which branch to use.
Okay, I can't implement this as a combo box in textual, but it will now
remember the branches you use for merging and will offer the last branch you
used as the default entry, and if you start typing the name of another branch,
it will show up as an option to auto-complete (with right-arrow key).
> > This is "as designed" right now -- the Signed-off-by and Link trailers are
> > applied to the merge commit and leave the individual commits untouched. It's a
> > controversial take, I know. :)
>
> > Would you prefer that we always applied Signed-off-by: and Link: to each
> > individual commit even if we're doing a merge?
>
> *Especially* if we're doing a merge, I think the general expectation is
> that there will be a signoff from the person who committed the patch -
> in -next we check every commit for both author and committer signoffs.
> If we cherry pick and they get applied then that'd work out OK, but if
> we merge and the unsigned commits end up in -next that'd be an issue.
> Missing signoffs will also mess up with flows like cherry picking
> patches back into stable, the merge would be long gone by the time the
> commit shows up in stable.
OK, this is now working exactly as with "b4 shazam" and has the functionality
you're looking for.
> > I'll push back on this a little simply because there's a lot that is keyed off
> > the number of patches in the series. If we cherry-pick before we even review,
> > this makes a lot of other functionality fragile. However, there is code now to
> > mark patches as "skip" (with x) -- this should let you completely ignore them.
> > When we "take" a series with ignored patches we will automatically default to
> > the cherry-pick strategy and pre-unselect the ignored patches. Would that come
> > kinda close to what you want?
>
> Possibly? I'd need to try it. Some of these serieses are quite large
> so even finding the relevant mails can be annoying.
Let me know if that's not sufficient. We can also give a way to hide skipped
patches so they aren't even in the picture.
> > I don't think I fixed this yet, but there's been churn in that code today. See
> > if it's still doing that.
>
> Yeah, still seems to be present.
This should be now fixed.
There's also new functionality in the current master:
- "take" is now decoupled from "accept" -- you can take into any branch and
the series won't be marked as "accepted" if you leave that checkbox
unchecked
- you can snooze a series, including until we find a specific tag in the local
tree; so, in theory, you could snooze a series until v7.0-rc3 or something
like that
Thanks for the feedback, and I'll work on improving the comment/review/email
UI next once I collect a bit more feedback from others.
Cheers,
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-03 18:21 ` Konstantin Ryabitsev
@ 2026-03-12 17:21 ` Alexandre Belloni
2026-03-13 15:42 ` Konstantin Ryabitsev
2026-03-12 17:35 ` Mark Brown
1 sibling, 1 reply; 69+ messages in thread
From: Alexandre Belloni @ 2026-03-12 17:21 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: Mark Brown, users, tools
On 03/03/2026 13:21:21-0500, Konstantin Ryabitsev wrote:
> > > I'll push back on this a little simply because there's a lot that is keyed off
> > > the number of patches in the series. If we cherry-pick before we even review,
> > > this makes a lot of other functionality fragile. However, there is code now to
> > > mark patches as "skip" (with x) -- this should let you completely ignore them.
> > > When we "take" a series with ignored patches we will automatically default to
> > > the cherry-pick strategy and pre-unselect the ignored patches. Would that come
> > > kinda close to what you want?
> >
> > Possibly? I'd need to try it. Some of these serieses are quite large
> > so even finding the relevant mails can be annoying.
>
> Let me know if that's not sufficient. We can also give a way to hide skipped
> patches so they aren't even in the picture.
This worked for me but thank-you still tried to thank for all the
commits of the series.
Also: I used cherry-pick, would Linear work?
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-12 17:21 ` Alexandre Belloni
@ 2026-03-13 15:42 ` Konstantin Ryabitsev
2026-03-13 15:55 ` Alexandre Belloni
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-13 15:42 UTC (permalink / raw)
To: Alexandre Belloni; +Cc: Mark Brown, users, tools
On Thu, Mar 12, 2026 at 06:21:56PM +0100, Alexandre Belloni wrote:
> > Let me know if that's not sufficient. We can also give a way to hide skipped
> > patches so they aren't even in the picture.
>
> This worked for me but thank-you still tried to thank for all the
> commits of the series.
This is fixed in the latest master.
> Also: I used cherry-pick, would Linear work?
It always uses linear for cherry-picked commits.
Thanks for testing!
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-13 15:42 ` Konstantin Ryabitsev
@ 2026-03-13 15:55 ` Alexandre Belloni
2026-03-21 10:01 ` Alexandre Belloni
0 siblings, 1 reply; 69+ messages in thread
From: Alexandre Belloni @ 2026-03-13 15:55 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: Mark Brown, users, tools
On 13/03/2026 11:42:12-0400, Konstantin Ryabitsev wrote:
> On Thu, Mar 12, 2026 at 06:21:56PM +0100, Alexandre Belloni wrote:
> > > Let me know if that's not sufficient. We can also give a way to hide skipped
> > > patches so they aren't even in the picture.
> >
> > This worked for me but thank-you still tried to thank for all the
> > commits of the series.
>
> This is fixed in the latest master.
>
> > Also: I used cherry-pick, would Linear work?
>
> It always uses linear for cherry-picked commits.
>
Thanks!
Maybe I should start a separate thread but since some time ago, b4 is
not able to change patch state on patchwork.ozlabs.org. I attributed
that to how they implemented anubis but b4 review is able to get the
patch list. It is simply not able to update the patch status. Are there
any logs that would help you?
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-13 15:55 ` Alexandre Belloni
@ 2026-03-21 10:01 ` Alexandre Belloni
0 siblings, 0 replies; 69+ messages in thread
From: Alexandre Belloni @ 2026-03-21 10:01 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: Mark Brown, users, tools
On 13/03/2026 16:55:01+0100, Alexandre Belloni wrote:
> On 13/03/2026 11:42:12-0400, Konstantin Ryabitsev wrote:
> > On Thu, Mar 12, 2026 at 06:21:56PM +0100, Alexandre Belloni wrote:
> > > > Let me know if that's not sufficient. We can also give a way to hide skipped
> > > > patches so they aren't even in the picture.
> > >
> > > This worked for me but thank-you still tried to thank for all the
> > > commits of the series.
> >
> > This is fixed in the latest master.
> >
> > > Also: I used cherry-pick, would Linear work?
> >
> > It always uses linear for cherry-picked commits.
> >
>
> Thanks!
>
> Maybe I should start a separate thread but since some time ago, b4 is
> not able to change patch state on patchwork.ozlabs.org. I attributed
> that to how they implemented anubis but b4 review is able to get the
> patch list. It is simply not able to update the patch status. Are there
> any logs that would help you?
I've tracked this down to the v1.2 api returning a 500 error on
patchwork.ozlab.org e.g.:
https://patchwork.ozlabs.org/api/1.1/patches/2212397/ works but
https://patchwork.ozlabs.org/api/1.2/patches/2212397/ doesn't.
I see the API version is hardcoded, is it enough to change it back to
1.1 or is the API too different for b4 to handle it?
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-03 18:21 ` Konstantin Ryabitsev
2026-03-12 17:21 ` Alexandre Belloni
@ 2026-03-12 17:35 ` Mark Brown
2026-03-13 15:42 ` Konstantin Ryabitsev
1 sibling, 1 reply; 69+ messages in thread
From: Mark Brown @ 2026-03-12 17:35 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 1203 bytes --]
On Tue, Mar 03, 2026 at 01:21:21PM -0500, Konstantin Ryabitsev wrote:
> On Tue, Mar 03, 2026 at 12:42:26PM +0000, Mark Brown wrote:
> > > I'll push back on this a little simply because there's a lot that is keyed off
> > > the number of patches in the series. If we cherry-pick before we even review,
> > > this makes a lot of other functionality fragile. However, there is code now to
> > > mark patches as "skip" (with x) -- this should let you completely ignore them.
> > > When we "take" a series with ignored patches we will automatically default to
> > > the cherry-pick strategy and pre-unselect the ignored patches. Would that come
> > > kinda close to what you want?
> > Possibly? I'd need to try it. Some of these serieses are quite large
> > so even finding the relevant mails can be annoying.
> Let me know if that's not sufficient. We can also give a way to hide skipped
> patches so they aren't even in the picture.
Having used this a bit I think for larger serieses hiding would be very
useful, and it would be useful to still be able to do a merge even with
skipped patches (common case is some DTS patches at the end of a series
which are basically unrelated to the main series).
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-12 17:35 ` Mark Brown
@ 2026-03-13 15:42 ` Konstantin Ryabitsev
0 siblings, 0 replies; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-13 15:42 UTC (permalink / raw)
To: Mark Brown; +Cc: users, tools
On Thu, Mar 12, 2026 at 05:35:13PM +0000, Mark Brown wrote:
> > Let me know if that's not sufficient. We can also give a way to hide skipped
> > patches so they aren't even in the picture.
>
> Having used this a bit I think for larger serieses hiding would be very
> useful, and it would be useful to still be able to do a merge even with
> skipped patches (common case is some DTS patches at the end of a series
> which are basically unrelated to the main series).
You can toggle-hide them with "H" in the latest master.
Cheers,
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-27 19:53 b4 review available in master Konstantin Ryabitsev
2026-02-28 15:12 ` Mark Brown
2026-02-28 15:53 ` Mark Brown
@ 2026-02-28 16:36 ` Conor Dooley
2026-02-28 16:45 ` Konstantin Ryabitsev
2026-02-28 18:31 ` Michael S. Tsirkin
` (6 subsequent siblings)
9 siblings, 1 reply; 69+ messages in thread
From: Conor Dooley @ 2026-02-28 16:36 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 28691 bytes --]
On Fri, Feb 27, 2026 at 02:53:07PM -0500, Konstantin Ryabitsev wrote:
> Hi, all:
>
> TLDR: "b4 review" is a terminal UI that hopes to streamline a lot of
> maintainer duties. A getting-started guide with screencasts is here:
> https://b4.docs.kernel.org/en/latest/reviewer/getting-started.html
>
> It's available from b4 master pending initial testing and will be generally
> available in b4-0.15.
>
> Longread:
>
> I've been working on "b4 review" for a while, with a few false-starts, a lot
> of despair, and then a whole new direction that seemed to have finally worked.
> So, the usual. :)
>
> I am finally feeling confident enough to release the code as initial
> technology preview, with the primary goal to get some direct feedback. The
> goal of the UI was to work alongside your usual email client, allowing you to
> use it for as much or as little as you like.
>
> The feature only exists in b4 master branch, so if you want to try it, you'll
> need to be comfortable running b4 from checkout. It also requires having a
> valid email server configured with git for sending outgoing mail. If you're
> already using "b4 ty" then you're good to go.
>
> The only new dependency introduced is python-textual. I say this with a bit of
> tongue-in-cheek, because textual pulls in a ton of other dependencies, but
> it's usually already installed on many modern distros anyway.
>
> You can do the following things:
>
> - Track incoming patch series from lore using a message-id, URL, change-id,
> or by piping a message from your mail client (e.g. via mutt integration)
> - TUI-based tracking list with series lifecycle management (new, reviewing,
> replied, taken, thanked, archived, waiting)
> - Split-pane review interface with patch list and syntax-highlighted diff view
> - Quick trailer addition (Reviewed-by, Acked-by, Tested-by) with a single
> keypress
> - Inline diff commenting via $EDITOR — comments are automatically formatted
> as proper quoted-reply emails for the mailing list
> - Preview emails to inspect exactly what will be sent before sending
> - Fetch and display follow-up messages from other reviewers directly in the
> diff view
> - Range-diff between series revisions without leaving the TUI
> - Automatic discovery of older and newer revisions when tracking a series
> - Revision upgrade that preserves your review comments on unchanged patches
> - Three merge strategies for applying series: merge commit, linear (git-am),
> and cherry-pick
> - Thank-you note generation listing each applied commit with its hash (and
> more reliably than b4 ty, since we keep track of applied series instead of
> guessing)
> - 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
This patchwork part is neat, this is definitely something that I'll play
with. Patchwork is great, but I really dislike all the jumping back and
forth between editor/website/mailer. One thing that immediately seemed like
it'd be neat is a shortcut from the patchwork pane that does track + review.
Worktree support is something I didn't expect to exist, but it does that
that's real helpful ;)
Also, I attempted to open patchwork while my b4.pw-project was set to
"devicetree" and got an error:
MarkupError: Expected markup style value (found '1/1] dt-bindings: net: can: nxp,sja1000: add reference to fsl,imx-weim-peripherals.yaml').
There's some massive splat I can send you if this does not reproduce.
I wanted to look into whether or not I could quickly reply to stuff from
the tui (since like 80% of binding patches don't require a checkout) and
encountered this smaller reproduction of the same thing.
$ /stuff/b4/b4.sh review track -i linux-dt D8CE6E5B1001D797+a6f36da0272ac18fff2a992976b1defc3bb01af6.1772289741.git.lv.zheng@linux.spacemit.com
< snip >
$ /stuff/b4/b4.sh review tui -i linux-dt
Retrieving series: 52E07C3A9B398459+126b1f464ae2a8bfcca6f504c64346541ae4db6f.1772289741.git.lv.zheng@linux.spacemit.com
Looking up https://lore.kernel.org/all/52E07C3A9B398459%2B126b1f464ae2a8bfcca6f504c64346541ae4db6f.1772289741.git.lv.zheng@linux.spacemit.com/
Grabbing thread from lore.kernel.org/all/52E07C3A9B398459%2B126b1f464ae2a8bfcca6f504c64346541ae4db6f.1772289741.git.lv.zheng@linux.spacemit.com/t.mbox.gz
Checking attestation on all messages, may take a moment...
---
✓ [PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support
✓ [PATCH v5 2/8] iommu/riscv: Add auxiliary bus framework and HPM device support
✓ [PATCH v5 3/8] iommu/riscv: Add HPM support for performance monitoring
✓ [PATCH v5 4/8] dt-bindings: iommu: Add spacemit/t100 features
✓ [PATCH v5 5/8] spacemit/t100: Add global filter awareness for RISC-V IOMMU HPM
✓ [PATCH v5 6/8] iommu/riscv: Add SpacemiT T100 IOATC HPM support
✓ [PATCH v5 7/8] iommu/riscv: Add vendor event support for RISC-V IOMMU HPM
✓ [PATCH v5 8/8] perf vendor events riscv: Add SpacemiT T100 HPM event aliases
---
✓ Signed: DKIM/linux.spacemit.com
Guessing base commit...
Base: tags/next-20260227 (best guess, 9/11 blobs matched)
Base: tags/next-20260227
Magic: Preparing a sparse worktree
---
Applying: iommu/riscv: Enable IOMMU DMA mapping support
Applying: iommu/riscv: Add auxiliary bus framework and HPM device support
Applying: iommu/riscv: Add HPM support for performance monitoring
Applying: dt-bindings: iommu: Add spacemit/t100 features
Applying: spacemit/t100: Add global filter awareness for RISC-V IOMMU HPM
Applying: iommu/riscv: Add SpacemiT T100 IOATC HPM support
Applying: iommu/riscv: Add vendor event support for RISC-V IOMMU HPM
Applying: perf vendor events riscv: Add SpacemiT T100 HPM event aliases
---
Fetching into FETCH_HEAD
Review branch b4/review/20260228-iommu-riscv-enable-iommu-dma-mapping-support-feee43387d2a created successfully
Review branch created: b4/review/20260228-iommu-riscv-enable-iommu-dma-mapping-support-feee43387d2a
Checking out branch and starting review UI...
╭────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Traceback (most recent call last) ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ /stuff/b4/src/b4/review_tui/_review_app.py:219 in on_mount │
│ │
│ 216 │ ╭─────────────────────────────────────────── locals ────────────────────────────────────────────╮ │
│ 217 │ def on_mount(self) -> None: │ self = ReviewApp(title='b4 review', classes={'-dark-mode'}, pseudo_classes={'focus', 'dark'}) │ │
│ 218 │ │ self._refresh_newer_warning() ╰───────────────────────────────────────────────────────────────────────────────────────────────╯ │
│ ❱ 219 │ │ self._refresh_title_bar() │
│ 220 │ │ self._populate_patch_list() │
│ 221 │ │ self._show_content(self._selected_idx) │
│ 222 │
│ │
│ /stuff/b4/src/b4/review_tui/_review_app.py:235 in _refresh_title_bar │
│ │
│ 232 │ │ else: ╭───────────────────────────────────────────── locals ─────────────────────────────────────────────╮ │
│ 233 │ │ │ label = f' \u270e {subject}' │ bar = Static(id='title-bar') │ │
│ 234 │ │ │ bar.styles.background = self.get_css_variables()['primary-darken-2'] │ label = ' ✎ [PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ ❱ 235 │ │ bar.update(label) │ self = ReviewApp(title='b4 review', classes={'-dark-mode'}, pseudo_classes={'focus', 'dark'}) │ │
│ 236 │ │ subject = '[PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ 237 │ def _refresh_newer_warning(self) -> None: ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ │
│ 238 │ │ """Show or hide the newer-version warning bar based on tracking data.""" │
│ │
│ /usr/lib/python3/dist-packages/textual/widgets/_static.py:106 in update │
│ │
│ 103 │ │ """ ╭────────────────────────────────── locals ───────────────────────────────────╮ │
│ 104 │ │ │ content = ' ✎ [PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ 105 │ │ self._content = content │ self = Static(id='title-bar') │ │
│ ❱ 106 │ │ self._visual = visualize(self, content, markup=self._render_markup) ╰─────────────────────────────────────────────────────────────────────────────╯ │
│ 107 │ │ self.refresh(layout=True) │
│ 108 │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
MarkupError: Expected markup style value (found '1/8] iommu/riscv: Enable IOMMU DMA mapping support').
NOTE: 1 of 2 errors shown. Run with textual run --dev to see all errors.
Checking out branch and starting review UI...
╭────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Traceback (most recent call last) ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ /stuff/b4/src/b4/review_tui/_review_app.py:219 in on_mount │
│ │
│ 216 │ ╭─────────────────────────────────────────── locals ────────────────────────────────────────────╮ │
│ 217 │ def on_mount(self) -> None: │ self = ReviewApp(title='b4 review', classes={'-dark-mode'}, pseudo_classes={'focus', 'dark'}) │ │
│ 218 │ │ self._refresh_newer_warning() ╰───────────────────────────────────────────────────────────────────────────────────────────────╯ │
│ ❱ 219 │ │ self._refresh_title_bar() │
│ 220 │ │ self._populate_patch_list() │
│ 221 │ │ self._show_content(self._selected_idx) │
│ 222 │
│ │
│ /stuff/b4/src/b4/review_tui/_review_app.py:235 in _refresh_title_bar │
│ │
│ 232 │ │ else: ╭───────────────────────────────────────────── locals ─────────────────────────────────────────────╮ │
│ 233 │ │ │ label = f' \u270e {subject}' │ bar = Static(id='title-bar') │ │
│ 234 │ │ │ bar.styles.background = self.get_css_variables()['primary-darken-2'] │ label = ' ✎ [PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ ❱ 235 │ │ bar.update(label) │ self = ReviewApp(title='b4 review', classes={'-dark-mode'}, pseudo_classes={'focus', 'dark'}) │ │
│ 236 │ │ subject = '[PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ 237 │ def _refresh_newer_warning(self) -> None: ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ │
│ 238 │ │ """Show or hide the newer-version warning bar based on tracking data.""" │
│ │
│ /usr/lib/python3/dist-packages/textual/widgets/_static.py:106 in update │
│ │
│ 103 │ │ """ ╭────────────────────────────────── locals ───────────────────────────────────╮ │
│ 104 │ │ │ content = ' ✎ [PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ 105 │ │ self._content = content │ self = Static(id='title-bar') │ │
│ ❱ 106 │ │ self._visual = visualize(self, content, markup=self._render_markup) ╰─────────────────────────────────────────────────────────────────────────────╯ │
│ 107 │ │ self.refresh(layout=True) │
│ 108 │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
MarkupError: Expected markup style value (found '1/8] iommu/riscv: Enable IOMMU DMA mapping support').
NOTE: 1 of 2 errors shown. Run with textual run --dev to see all errors
This seems promising though, could definitely see this making my life
easier, with less hoping between patchwork/mutt/git repo.
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-28 16:36 ` Conor Dooley
@ 2026-02-28 16:45 ` Konstantin Ryabitsev
2026-02-28 16:48 ` Conor Dooley
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-02-28 16:45 UTC (permalink / raw)
To: Conor Dooley; +Cc: users, tools
On Sat, Feb 28, 2026 at 04:36:07PM +0000, Conor Dooley wrote:
> Also, I attempted to open patchwork while my b4.pw-project was set to
> "devicetree" and got an error:
> MarkupError: Expected markup style value (found '1/1] dt-bindings: net: can: nxp,sja1000: add reference to fsl,imx-weim-peripherals.yaml').
> There's some massive splat I can send you if this does not reproduce.
Bah, the problem is that textual treats [] as style markup, so there's a bunch
of places where we have to escape that. I expect this will come up routinely
as more places requiring escaping are found.
I put a fix into latest master for this particular one. Thanks!
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-28 16:45 ` Konstantin Ryabitsev
@ 2026-02-28 16:48 ` Conor Dooley
2026-02-28 16:57 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Conor Dooley @ 2026-02-28 16:48 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 12145 bytes --]
On Sat, Feb 28, 2026 at 11:45:58AM -0500, Konstantin Ryabitsev wrote:
> On Sat, Feb 28, 2026 at 04:36:07PM +0000, Conor Dooley wrote:
> > Also, I attempted to open patchwork while my b4.pw-project was set to
> > "devicetree" and got an error:
> > MarkupError: Expected markup style value (found '1/1] dt-bindings: net: can: nxp,sja1000: add reference to fsl,imx-weim-peripherals.yaml').
> > There's some massive splat I can send you if this does not reproduce.
>
> Bah, the problem is that textual treats [] as style markup, so there's a bunch
> of places where we have to escape that. I expect this will come up routinely
> as more places requiring escaping are found.
>
> I put a fix into latest master for this particular one. Thanks!
Seems like you got half the problem? Certainly predictive on the "more
places though! ;)
$ git checkout korg/master
Previous HEAD position was a00f452 review: drop Binding.Group to support older textual
HEAD is now at a21b8a4 review: escape markup in title bar subject
$ popd
/stuff/linux-dt /stuff/linux
$ /stuff/b4/b4.sh review tui -i linux-dt
╭────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Traceback (most recent call last) ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ /stuff/b4/src/b4/review_tui/_review_app.py:219 in on_mount │
│ │
│ 216 │ ╭─────────────────────────────────────────── locals ────────────────────────────────────────────╮ │
│ 217 │ def on_mount(self) -> None: │ self = ReviewApp(title='b4 review', classes={'-dark-mode'}, pseudo_classes={'dark', 'focus'}) │ │
│ 218 │ │ self._refresh_newer_warning() ╰───────────────────────────────────────────────────────────────────────────────────────────────╯ │
│ ❱ 219 │ │ self._refresh_title_bar() │
│ 220 │ │ self._populate_patch_list() │
│ 221 │ │ self._show_content(self._selected_idx) │
│ 222 │
│ │
│ /stuff/b4/src/b4/review_tui/_review_app.py:235 in _refresh_title_bar │
│ │
│ 232 │ │ else: ╭───────────────────────────────────────────── locals ─────────────────────────────────────────────╮ │
│ 233 │ │ │ label = f' \u270e {subject}' │ bar = Static(id='title-bar') │ │
│ 234 │ │ │ bar.styles.background = self.get_css_variables()['primary-darken-2'] │ label = ' ✎ [PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ ❱ 235 │ │ bar.update(label) │ self = ReviewApp(title='b4 review', classes={'-dark-mode'}, pseudo_classes={'dark', 'focus'}) │ │
│ 236 │ │ subject = '[PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ 237 │ def _refresh_newer_warning(self) -> None: ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ │
│ 238 │ │ """Show or hide the newer-version warning bar based on tracking data.""" │
│ │
│ /usr/lib/python3/dist-packages/textual/widgets/_static.py:106 in update │
│ │
│ 103 │ │ """ ╭────────────────────────────────── locals ───────────────────────────────────╮ │
│ 104 │ │ │ content = ' ✎ [PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ 105 │ │ self._content = content │ self = Static(id='title-bar') │ │
│ ❱ 106 │ │ self._visual = visualize(self, content, markup=self._render_markup) ╰─────────────────────────────────────────────────────────────────────────────╯ │
│ 107 │ │ self.refresh(layout=True) │
│ 108 │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
MarkupError: Expected markup style value (found '1/8] iommu/riscv: Enable IOMMU DMA mapping support').
NOTE: 1 of 2 errors shown. Run with textual run --dev to see all errors.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-28 16:48 ` Conor Dooley
@ 2026-02-28 16:57 ` Konstantin Ryabitsev
2026-02-28 17:00 ` Conor Dooley
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-02-28 16:57 UTC (permalink / raw)
To: Conor Dooley; +Cc: users, tools
On Sat, Feb 28, 2026 at 04:48:58PM +0000, Conor Dooley wrote:
> > I put a fix into latest master for this particular one. Thanks!
>
> Seems like you got half the problem? Certainly predictive on the "more
> places though! ;)
Indeed. I put the escaping into a bunch of other places, too, where we can
reasonably expect [] syntax from subject prefixes and such. Try it out in the
latest master.
Thanks!
-K
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-28 16:57 ` Konstantin Ryabitsev
@ 2026-02-28 17:00 ` Conor Dooley
2026-02-28 17:05 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Conor Dooley @ 2026-02-28 17:00 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 11682 bytes --]
On Sat, Feb 28, 2026 at 11:57:09AM -0500, Konstantin Ryabitsev wrote:
> On Sat, Feb 28, 2026 at 04:48:58PM +0000, Conor Dooley wrote:
> > > I put a fix into latest master for this particular one. Thanks!
> >
> > Seems like you got half the problem? Certainly predictive on the "more
> > places though! ;)
>
> Indeed. I put the escaping into a bunch of other places, too, where we can
> reasonably expect [] syntax from subject prefixes and such. Try it out in the
> latest master.
$ /stuff/b4/b4.sh review tui -i linux-dt
╭────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────── Traceback (most recent call last) ───────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╮
│ /stuff/b4/src/b4/review_tui/_review_app.py:219 in on_mount │
│ │
│ 216 │ ╭─────────────────────────────────────────── locals ────────────────────────────────────────────╮ │
│ 217 │ def on_mount(self) -> None: │ self = ReviewApp(title='b4 review', classes={'-dark-mode'}, pseudo_classes={'dark', 'focus'}) │ │
│ 218 │ │ self._refresh_newer_warning() ╰───────────────────────────────────────────────────────────────────────────────────────────────╯ │
│ ❱ 219 │ │ self._refresh_title_bar() │
│ 220 │ │ self._populate_patch_list() │
│ 221 │ │ self._show_content(self._selected_idx) │
│ 222 │
│ │
│ /stuff/b4/src/b4/review_tui/_review_app.py:235 in _refresh_title_bar │
│ │
│ 232 │ │ else: ╭───────────────────────────────────────────── locals ─────────────────────────────────────────────╮ │
│ 233 │ │ │ label = f' \u270e {subject}' │ bar = Static(id='title-bar') │ │
│ 234 │ │ │ bar.styles.background = self.get_css_variables()['primary-darken-2'] │ label = ' ✎ [PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ ❱ 235 │ │ bar.update(label) │ self = ReviewApp(title='b4 review', classes={'-dark-mode'}, pseudo_classes={'dark', 'focus'}) │ │
│ 236 │ │ subject = '[PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ 237 │ def _refresh_newer_warning(self) -> None: ╰──────────────────────────────────────────────────────────────────────────────────────────────────╯ │
│ 238 │ │ """Show or hide the newer-version warning bar based on tracking data.""" │
│ │
│ /usr/lib/python3/dist-packages/textual/widgets/_static.py:106 in update │
│ │
│ 103 │ │ """ ╭────────────────────────────────── locals ───────────────────────────────────╮ │
│ 104 │ │ │ content = ' ✎ [PATCH v5 1/8] iommu/riscv: Enable IOMMU DMA mapping support' │ │
│ 105 │ │ self._content = content │ self = Static(id='title-bar') │ │
│ ❱ 106 │ │ self._visual = visualize(self, content, markup=self._render_markup) ╰─────────────────────────────────────────────────────────────────────────────╯ │
│ 107 │ │ self.refresh(layout=True) │
│ 108 │
╰────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────╯
MarkupError: Expected markup style value (found '1/8] iommu/riscv: Enable IOMMU DMA mapping support').
NOTE: 1 of 2 errors shown. Run with textual run --dev to see all errors.
Am I just cursed? This is on 9ca5b4f review: escape remaining user strings passed to markup-enabled widgets
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-28 17:00 ` Conor Dooley
@ 2026-02-28 17:05 ` Konstantin Ryabitsev
2026-02-28 17:12 ` Conor Dooley
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-02-28 17:05 UTC (permalink / raw)
To: Conor Dooley; +Cc: users, tools
On Sat, Feb 28, 2026 at 05:00:10PM +0000, Conor Dooley wrote:
> Am I just cursed? This is on 9ca5b4f review: escape remaining user strings passed to markup-enabled widgets
Did you remember to completely exit the running app and restart it? I swear,
this should be fixed.
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-28 17:05 ` Konstantin Ryabitsev
@ 2026-02-28 17:12 ` Conor Dooley
2026-02-28 17:21 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Conor Dooley @ 2026-02-28 17:12 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 405 bytes --]
On Sat, Feb 28, 2026 at 12:05:57PM -0500, Konstantin Ryabitsev wrote:
> On Sat, Feb 28, 2026 at 05:00:10PM +0000, Conor Dooley wrote:
> > Am I just cursed? This is on 9ca5b4f review: escape remaining user strings passed to markup-enabled widgets
>
> Did you remember to completely exit the running app and restart it? I swear,
> this should be fixed.
Yeah, not running according to ps anyway.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-28 17:12 ` Conor Dooley
@ 2026-02-28 17:21 ` Konstantin Ryabitsev
2026-02-28 17:34 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-02-28 17:21 UTC (permalink / raw)
To: Conor Dooley; +Cc: users, tools
On Sat, Feb 28, 2026 at 05:12:51PM +0000, Conor Dooley wrote:
> > Did you remember to completely exit the running app and restart it? I swear,
> > this should be fixed.
>
>
> Yeah, not running according to ps anyway.
Argh, escape_markup will, unhelpfully, only escape actual markup. So, it will
escape [bold] into \[bold\] but leave [PATCH] as [PATCH]. WTF, who decided
this was a good idea?
I'll commit a more wholesome fix shortly, sorry about that.
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-28 17:21 ` Konstantin Ryabitsev
@ 2026-02-28 17:34 ` Konstantin Ryabitsev
2026-02-28 18:37 ` Conor Dooley
2026-02-28 22:16 ` Conor Dooley
0 siblings, 2 replies; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-02-28 17:34 UTC (permalink / raw)
To: Conor Dooley; +Cc: users, tools
On Sat, Feb 28, 2026 at 12:21:27PM -0500, Konstantin Ryabitsev wrote:
> Argh, escape_markup will, unhelpfully, only escape actual markup. So, it will
> escape [bold] into \[bold\] but leave [PATCH] as [PATCH]. WTF, who decided
> this was a good idea?
>
> I'll commit a more wholesome fix shortly, sorry about that.
OK, the more wholesome fix is in place now. Please try it out and hopefully
that lifts the curse.
Thanks!
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-28 17:34 ` Konstantin Ryabitsev
@ 2026-02-28 18:37 ` Conor Dooley
2026-02-28 22:16 ` Conor Dooley
1 sibling, 0 replies; 69+ messages in thread
From: Conor Dooley @ 2026-02-28 18:37 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 582 bytes --]
On Sat, Feb 28, 2026 at 12:34:40PM -0500, Konstantin Ryabitsev wrote:
> On Sat, Feb 28, 2026 at 12:21:27PM -0500, Konstantin Ryabitsev wrote:
> > Argh, escape_markup will, unhelpfully, only escape actual markup. So, it will
> > escape [bold] into \[bold\] but leave [PATCH] as [PATCH]. WTF, who decided
> > this was a good idea?
> >
> > I'll commit a more wholesome fix shortly, sorry about that.
>
> OK, the more wholesome fix is in place now. Please try it out and hopefully
> that lifts the curse.
Looks like it did, I'll have a play around tomorrow so. Thanks :)
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-28 17:34 ` Konstantin Ryabitsev
2026-02-28 18:37 ` Conor Dooley
@ 2026-02-28 22:16 ` Conor Dooley
2026-02-28 22:32 ` Conor Dooley
1 sibling, 1 reply; 69+ messages in thread
From: Conor Dooley @ 2026-02-28 22:16 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 1276 bytes --]
On Sat, Feb 28, 2026 at 12:34:40PM -0500, Konstantin Ryabitsev wrote:
> On Sat, Feb 28, 2026 at 12:21:27PM -0500, Konstantin Ryabitsev wrote:
> > Argh, escape_markup will, unhelpfully, only escape actual markup. So, it will
> > escape [bold] into \[bold\] but leave [PATCH] as [PATCH]. WTF, who decided
> > this was a good idea?
> >
> > I'll commit a more wholesome fix shortly, sorry about that.
>
> OK, the more wholesome fix is in place now. Please try it out and hopefully
> that lifts the curse.
Yeah, got me further but I got stuck again - although not on a crash. I
sent you a mail with a screenshot of where I am :)
One thing I noticed was missing was that I have no idea while looking at
a patch who sent it. If you look at the mail I sent with the screenshot:
Lv Zheng is the submitter, so the signoff chain is wrong (cos there's no
from header in the patch to make Jingyu the author, so either there's a
co-developed-by missing or a from header is needed), but it's impossible
to tell from this UI. Once you hit "reply", then you can see it, but not
from the main view.
I have some other thoughts that are half baked, and I will try to
elaborate on them once I have at least tried to do a days review with
the tool.
Cheers,
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-28 22:16 ` Conor Dooley
@ 2026-02-28 22:32 ` Conor Dooley
2026-03-03 5:16 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Conor Dooley @ 2026-02-28 22:32 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 2042 bytes --]
On Sat, Feb 28, 2026 at 10:16:15PM +0000, Conor Dooley wrote:
> On Sat, Feb 28, 2026 at 12:34:40PM -0500, Konstantin Ryabitsev wrote:
> > On Sat, Feb 28, 2026 at 12:21:27PM -0500, Konstantin Ryabitsev wrote:
> > > Argh, escape_markup will, unhelpfully, only escape actual markup. So, it will
> > > escape [bold] into \[bold\] but leave [PATCH] as [PATCH]. WTF, who decided
> > > this was a good idea?
> > >
> > > I'll commit a more wholesome fix shortly, sorry about that.
> >
> > OK, the more wholesome fix is in place now. Please try it out and hopefully
> > that lifts the curse.
>
> Yeah, got me further but I got stuck again - although not on a crash. I
> sent you a mail with a screenshot of where I am :)
>
> One thing I noticed was missing was that I have no idea while looking at
> a patch who sent it. If you look at the mail I sent with the screenshot:
> Lv Zheng is the submitter, so the signoff chain is wrong (cos there's no
> from header in the patch to make Jingyu the author, so either there's a
> co-developed-by missing or a from header is needed), but it's impossible
> to tell from this UI. Once you hit "reply", then you can see it, but not
> from the main view.
>
> I have some other thoughts that are half baked, and I will try to
> elaborate on them once I have at least tried to do a days review with
> the tool.
I found another issue. I tried to use it to send an email:
https://lore.kernel.org/all/177231592262.848068.18080490567217057666@spud/
The tool decided to send it from "Conor Dooley <conor.dooley@microchip.com>"
by that's not the from field in my gitconfig, but it is what my
user.email is:
| $ git config get sendemail.from
| Conor Dooley <conor@kernel.org>
| $ git config get user.email
| conor.dooley@microchip.com
Obviously this caused a bunch of rejections due to DMARC issues, because
it used the kernel.org email server setup from my gitconfig with my work
address.
I feel like I am spamming you with responses here, sorry about that!
Conor.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-28 22:32 ` Conor Dooley
@ 2026-03-03 5:16 ` Konstantin Ryabitsev
2026-03-04 21:38 ` Conor Dooley
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-03 5:16 UTC (permalink / raw)
To: Conor Dooley; +Cc: users, tools
On Sat, Feb 28, 2026 at 10:32:04PM +0000, Conor Dooley wrote:
> I found another issue. I tried to use it to send an email:
> https://lore.kernel.org/all/177231592262.848068.18080490567217057666@spud/
> The tool decided to send it from "Conor Dooley <conor.dooley@microchip.com>"
> by that's not the from field in my gitconfig, but it is what my
> user.email is:
>
> | $ git config get sendemail.from
> | Conor Dooley <conor@kernel.org>
> | $ git config get user.email
> | conor.dooley@microchip.com
>
> Obviously this caused a bunch of rejections due to DMARC issues, because
> it used the kernel.org email server setup from my gitconfig with my work
> address.
Thanks, it should be fixed now!
> I feel like I am spamming you with responses here, sorry about that!
Please continue! There's also a hopefully-noticeable notification when you're
running the review tui from a review branch that will tell you how to get back
to the series tracking view. :)
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-03 5:16 ` Konstantin Ryabitsev
@ 2026-03-04 21:38 ` Conor Dooley
2026-03-04 22:40 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Conor Dooley @ 2026-03-04 21:38 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 2901 bytes --]
On Tue, Mar 03, 2026 at 12:16:18AM -0500, Konstantin Ryabitsev wrote:
> On Sat, Feb 28, 2026 at 10:32:04PM +0000, Conor Dooley wrote:
> > I feel like I am spamming you with responses here, sorry about that!
>
> Please continue! There's also a hopefully-noticeable notification when you're
> running the review tui from a review branch that will tell you how to get back
> to the series tracking view. :)
I tried again today to use it but ran into some things flow wise, that
prevented me from actually doing any reviewing.
Firstly, with the patchwork pane, how do I determine which states
appear? Most things get set by a bot to "needs ack" in the dt patchwork,
but I only see things as "under review" or "new" in the pane. We use
"under review" to mean the bot is looking at it mainly and "needs ack"
for stuff that humans need to look at, so I need to figure out how to
display things with that status. Pretty much this is "your default view
doesn't match the default patchwork view (State = Action Required)".
Secondly, if I'm working from a patchwork, it'd be really great if I
could reply to patches directly from the patchwork pane. 90% of patches
I review (or more) I do not need to apply, so working in mutt is
significantly faster than having to track, then open in a different
pane. Related to that, being able to display the patchwork pane
per-patch as an option would be a wishlist item to make this replace
mutt for me.
That's from a dt-bindings review perspective though, how these things
work is fine (bugs aside) where I am the person applying patches. It's
probably far from the end of the world if dt-binding stuff stays in
mutt, since it's probably atypical.
Oh, and I managed to trigger a formatting bug again, on 0.15-dev-54639
MissingStyle: Failed to get style 'v2,1/2'; unable to parse 'v2,1/2' as color; 'v2,1/2' is not a valid color
after grabbing a series from patchwork
/stuff/b4/b4.sh review tui -i linux-dt
Retrieving series: <20260212082105.1878720-2-sh86.bae@samsung.com>
It creates a massive spam of text, but it contains:
text = <text 'Started tracking: dt-bindings: soc: samsung: exynos-sysreg: Add hsi0 for ExynosAutov920' [Span(18, 88, 'v2,1/2')] ''>
LMK if you require the full log for this.
Pretty sure I replicated it on 0.15-dev-18f8f too, but I'm not entirely
sure as I cannot untrack series to retest.
I also hit some weird interaction, where after hitting track on a load
of stuff in the patchwork pane, nothing shows up in my review pane, but
it looks like it should have appeared:
/stuff/b4/b4.sh review tui -i linux-dt
Retrieving series: <20260220-ayn-vendor-v1-1-292cbbb682b3@gmail.com>
Looking up https://lore.kernel.org/all/20260220-ayn-vendor-v1-1-292cbbb682b3@gmail.com/
Grabbing thread from lore.kernel.org/all/20260220-ayn-vendor-v1-1-292cbbb682b3@gmail.com/t.mbox.gz
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-04 21:38 ` Conor Dooley
@ 2026-03-04 22:40 ` Konstantin Ryabitsev
2026-03-04 22:55 ` Conor Dooley
2026-03-05 6:17 ` Konstantin Ryabitsev
0 siblings, 2 replies; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-04 22:40 UTC (permalink / raw)
To: Conor Dooley; +Cc: users, tools
On Wed, Mar 04, 2026 at 09:38:09PM +0000, Conor Dooley wrote:
> I tried again today to use it but ran into some things flow wise, that
> prevented me from actually doing any reviewing.
Thank you for being patient!
> Firstly, with the patchwork pane, how do I determine which states
> appear?
Whatever patchwork says is "action required". Would it help to be able to
filter/sort by state?
> Most things get set by a bot to "needs ack" in the dt patchwork,
> but I only see things as "under review" or "new" in the pane. We use
> "under review" to mean the bot is looking at it mainly and "needs ack"
> for stuff that humans need to look at, so I need to figure out how to
> display things with that status. Pretty much this is "your default view
> doesn't match the default patchwork view (State = Action Required)".
Ah, the difficulty is that patchwork doesn't export custom states or if they
are "action required" or not, so we have to go by defaults. Would it help to
have a configurable way to define which states we should show you and which we
shouldn't? E.g.:
[b4]
pw-hide-states = needs-ack,some-other-state
I'm hoping we can get patchwork to have an API call to show custom states,
since they can be configured per-instance.
> Secondly, if I'm working from a patchwork, it'd be really great if I
> could reply to patches directly from the patchwork pane. 90% of patches
> I review (or more) I do not need to apply, so working in mutt is
> significantly faster than having to track, then open in a different
> pane. Related to that, being able to display the patchwork pane
> per-patch as an option would be a wishlist item to make this replace
> mutt for me.
OK, noted, I'll think about how we can make this easier -- like a pre-review
state in both tracking app and patchwork app that lets you view the patches
and follow-ups and reply, but not do a more in-depth review where you have to
apply the series locally.
> Oh, and I managed to trigger a formatting bug again, on 0.15-dev-54639
/o\
> MissingStyle: Failed to get style 'v2,1/2'; unable to parse 'v2,1/2' as color; 'v2,1/2' is not a valid color
>
> after grabbing a series from patchwork
>
> /stuff/b4/b4.sh review tui -i linux-dt
> Retrieving series: <20260212082105.1878720-2-sh86.bae@samsung.com>
>
> It creates a massive spam of text, but it contains:
> text = <text 'Started tracking: dt-bindings: soc: samsung: exynos-sysreg: Add hsi0 for ExynosAutov920' [Span(18, 88, 'v2,1/2')] ''>
>
> LMK if you require the full log for this.
I know it's a pain to get textual's backtraces because they are not easy to
copy-paste, so if you just want to screenshot the bottom of that dump and post
a link to it, I'll be able to untangle it from there, usually.
I'm not able to replicate this if I do "b4 review track", so this is something
specific to the patchwork app, I think -- but seeing the trace will help.
> Pretty sure I replicated it on 0.15-dev-18f8f too, but I'm not entirely
> sure as I cannot untrack series to retest.
You can, just abandon it from the tracking app and it should be available to
track again from the patchwork app.
> I also hit some weird interaction, where after hitting track on a load
> of stuff in the patchwork pane, nothing shows up in my review pane, but
> it looks like it should have appeared:
Yeah, I'll need a bit more to go on here, but if you pulled in the latest
master in-between, there *are* database schema changes in recent commits. In
theory, rerunning "b4 review tui" should upgrade your db behind the scenes,
but this could have bugs in itself.
For the moment, if things enter a weird state, you can just delete
~/.local/share/b4/review/[identifier].sqlite3 and .git/b4-review/metadata.json
and then run "b4 review enroll" again. Hopefully, we'll stop upgrading db
schema all the time soon.
Thanks again!
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-03-04 22:40 ` Konstantin Ryabitsev
@ 2026-03-04 22:55 ` Conor Dooley
2026-03-05 3:26 ` Konstantin Ryabitsev
2026-03-05 6:17 ` Konstantin Ryabitsev
1 sibling, 1 reply; 69+ messages in thread
From: Conor Dooley @ 2026-03-04 22:55 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 1325 bytes --]
On Wed, Mar 04, 2026 at 05:40:17PM -0500, Konstantin Ryabitsev wrote:
> On Wed, Mar 04, 2026 at 09:38:09PM +0000, Conor Dooley wrote:
> > I tried again today to use it but ran into some things flow wise, that
> > prevented me from actually doing any reviewing.
> > Oh, and I managed to trigger a formatting bug again, on 0.15-dev-54639
>
> /o\
>
> > MissingStyle: Failed to get style 'v2,1/2'; unable to parse 'v2,1/2' as color; 'v2,1/2' is not a valid color
> >
> > after grabbing a series from patchwork
> >
> > /stuff/b4/b4.sh review tui -i linux-dt
> > Retrieving series: <20260212082105.1878720-2-sh86.bae@samsung.com>
> >
> > It creates a massive spam of text, but it contains:
> > text = <text 'Started tracking: dt-bindings: soc: samsung: exynos-sysreg: Add hsi0 for ExynosAutov920' [Span(18, 88, 'v2,1/2')] ''>
> >
> > LMK if you require the full log for this.
>
> I know it's a pain to get textual's backtraces because they are not easy to
> copy-paste, so if you just want to screenshot the bottom of that dump and post
> a link to it, I'll be able to untangle it from there, usually.
https://i.nuuls.com/wLaX3.png
> I'm not able to replicate this if I do "b4 review track", so this is something
> specific to the patchwork app, I think -- but seeing the trace will help.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-04 22:55 ` Conor Dooley
@ 2026-03-05 3:26 ` Konstantin Ryabitsev
0 siblings, 0 replies; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-05 3:26 UTC (permalink / raw)
To: Conor Dooley; +Cc: users, tools
On Wed, Mar 04, 2026 at 10:55:27PM +0000, Conor Dooley wrote:
> > I know it's a pain to get textual's backtraces because they are not easy to
> > copy-paste, so if you just want to screenshot the bottom of that dump and post
> > a link to it, I'll be able to untangle it from there, usually.
>
> https://i.nuuls.com/wLaX3.png
Thanks, this should be now fixed in master. Hopefully, not many more spots
with this bug left.
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-04 22:40 ` Konstantin Ryabitsev
2026-03-04 22:55 ` Conor Dooley
@ 2026-03-05 6:17 ` Konstantin Ryabitsev
1 sibling, 0 replies; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-05 6:17 UTC (permalink / raw)
To: Conor Dooley; +Cc: users, tools
On Wed, Mar 04, 2026 at 05:40:17PM -0500, Konstantin Ryabitsev wrote:
> > Secondly, if I'm working from a patchwork, it'd be really great if I
> > could reply to patches directly from the patchwork pane. 90% of patches
> > I review (or more) I do not need to apply, so working in mutt is
> > significantly faster than having to track, then open in a different
> > pane. Related to that, being able to display the patchwork pane
> > per-patch as an option would be a wishlist item to make this replace
> > mutt for me.
>
> OK, noted, I'll think about how we can make this easier -- like a pre-review
> state in both tracking app and patchwork app that lets you view the patches
> and follow-ups and reply, but not do a more in-depth review where you have to
> apply the series locally.
OK, the latest master completely revamps the "view" screen -- it's more like a
mutt experience. You can see the thread with all follow-ups and reply from
within that modal.
It's not quite complete -- we're not setting a "Replied" flag or inject your
reply into the thread until it comes back from the lore, but it's something to
poke at already. Let me know if this is more like what you were looking for.
-K
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-27 19:53 b4 review available in master Konstantin Ryabitsev
` (2 preceding siblings ...)
2026-02-28 16:36 ` Conor Dooley
@ 2026-02-28 18:31 ` Michael S. Tsirkin
2026-02-28 20:06 ` Konstantin Ryabitsev
2026-03-02 9:30 ` Michael S. Tsirkin
` (5 subsequent siblings)
9 siblings, 1 reply; 69+ messages in thread
From: Michael S. Tsirkin @ 2026-02-28 18:31 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
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
>
> Watch the screencasts in the link above if you're curious and please try it
> out. I'm sure there is a ton of corner cases that are not properly handled
> right now, so the more feedback I receive, the better.
>
> If you want to play with LLM reviews, you will need to specifically configure
> your agent. See docs for that. If you don't have a subscription available, I
> am told that there is a program for OSS maintainers available:
>
> https://claude.com/contact-sales/claude-for-oss
>
> If you know of a similar program offered by other LLM providers, I'll be happy
> to know about it. I've tested claude, gemini-cli and codex. (I did test using
> ollama-based agents a while back, but the results were a very mixed bag --
> there is usually not enough context available to these models to perform a
> useful code review, but ymmv.)
>
> Best regards,
> --
> KR
Amazing, thanks for working on this.
But I could not get it to work with gemini:
I have:
[b4]
review-agent-command = gemini --sandbox --allowed-tools 'Bash(git:*) Read Glob Grep Write(.git/b4-review/**) Edit(.git/b4-review/**)'
review-agent-prompt-path = .git/agent-reviewer.md
and this runs:
gemini --sandbox --allowed-tools Bash(git:*) Read Glob Grep Write(.git/b4-review/**) Edit(.git/b4-review/**) -- Read the prompt from /scm/b4/.git/agent-reviewer.md
But this just gets me an interactive prompt:
Running review agent: gemini --sandbox --allowed-tools Bash(git:*) Read Glob Grep Write(.git/b4-review/**) Edit(.git/b4-review/**) -- Read the prompt from /scm/b4/.git/agent-reviewer.md
███ █████████ ██████████ ██████ ██████ █████ ██████ █████ █████
░░░███ ███░░░░░███░░███░░░░░█░░██████ ██████ ░░███ ░░██████ ░░███ ░░███
░░░███ ███ ░░░ ░███ █ ░ ░███░█████░███ ░███ ░███░███ ░███ ░███
░░░███ ░███ ░██████ ░███░░███ ░███ ░███ ░███░░███░███ ░███
███░ ░███ █████ ░███░░█ ░███ ░░░ ░███ ░███ ░███ ░░██████ ░███
███░ ░░███ ░░███ ░███ ░ █ ░███ ░███ ░███ ░███ ░░█████ ░███
███░ ░░█████████ ██████████ █████ █████ █████ █████ ░░█████ █████
░░░ ░░░░░░░░░ ░░░░░░░░░░ ░░░░░ ░░░░░ ░░░░░ ░░░░░ ░░░░░ ░░░░░
Authenticated with gemini-api-key /auth
Tips for getting started:
1. Ask questions, edit files, or run commands.
2. Be specific for the best results.
3. Create GEMINI.md files to customize your interactions with Gemini.
4. /help for more information.
⚠ Warning: --allowed-tools cli argument and tools.allowed in settings.json are deprecated and will be removed in 1.0: Migrate to Policy
Engine: https://geminicli.com/docs/core/policy-engine/
? for shortcuts
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
shift+tab to accept edits 1 skill
▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀▀
> Type your message or @path/to/file
▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄▄
/scm/b4 (b4/review/20251013-b4-sparse-checkout-related-fixes-bad772cc2a80*) sandbox-0.31.0-2 /model Auto (Gemini 3)
Thanks!
--
MST
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-28 18:31 ` Michael S. Tsirkin
@ 2026-02-28 20:06 ` Konstantin Ryabitsev
2026-02-28 20:23 ` Michael S. Tsirkin
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-02-28 20:06 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: users, tools
On Sat, Feb 28, 2026 at 01:31:04PM -0500, Michael S. Tsirkin wrote:
> Amazing, thanks for working on this.
>
>
> But I could not get it to work with gemini:
> I have:
>
> [b4]
> review-agent-command = gemini --sandbox --allowed-tools 'Bash(git:*) Read Glob Grep Write(.git/b4-review/**) Edit(.git/b4-review/**)'
> review-agent-prompt-path = .git/agent-reviewer.md
Thanks for testing it out. My most salient recent experience with gemini-cli
is staring at:
Trying to reach gemini-flash-3-preview (Attempt 6/10)
and
Agent reached max turns limit (10)
(Sorry, Google people -- just stating facts.)
However, I know the following things:
1. Gemini really doesn't like reading from the .git dir of the repository.
2. It also doesn't like writing to that dir.
3. In fact, it only likes writing to the toplevel of the git dir you're in,
and into ~/.gemini/tmp/dirname/
Gemini itself just recommended me to run with "gemini --approval-mode=yolo"
which seems like a pretty terrible ideas. However, I'm now looking at more
RESOURCE_EXHAUSTED stack traces from gemini, so cannot test this further. If
someone with a more reliable access to gemini-cli can recommend what is a
better approach, I'm happy to implement it, otherwise I'll try again later
when Gemini is less cranky.
-K
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-28 20:06 ` Konstantin Ryabitsev
@ 2026-02-28 20:23 ` Michael S. Tsirkin
2026-02-28 20:37 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Michael S. Tsirkin @ 2026-02-28 20:23 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
On Sat, Feb 28, 2026 at 03:06:34PM -0500, Konstantin Ryabitsev wrote:
> On Sat, Feb 28, 2026 at 01:31:04PM -0500, Michael S. Tsirkin wrote:
> > Amazing, thanks for working on this.
> >
> >
> > But I could not get it to work with gemini:
> > I have:
> >
> > [b4]
> > review-agent-command = gemini --sandbox --allowed-tools 'Bash(git:*) Read Glob Grep Write(.git/b4-review/**) Edit(.git/b4-review/**)'
> > review-agent-prompt-path = .git/agent-reviewer.md
>
> Thanks for testing it out. My most salient recent experience with gemini-cli
> is staring at:
>
> Trying to reach gemini-flash-3-preview (Attempt 6/10)
>
> and
>
> Agent reached max turns limit (10)
>
> (Sorry, Google people -- just stating facts.)
>
> However, I know the following things:
>
> 1. Gemini really doesn't like reading from the .git dir of the repository.
> 2. It also doesn't like writing to that dir.
> 3. In fact, it only likes writing to the toplevel of the git dir you're in,
> and into ~/.gemini/tmp/dirname/
>
> Gemini itself just recommended me to run with "gemini --approval-mode=yolo"
> which seems like a pretty terrible ideas.
On that, I'll send another message)
> However, I'm now looking at more
> RESOURCE_EXHAUSTED stack traces from gemini, so cannot test this further. If
> someone with a more reliable access to gemini-cli can recommend what is a
> better approach, I'm happy to implement it, otherwise I'll try again later
> when Gemini is less cranky.
>
> -K
The following seems to fix it.
I am also testing different models and some of them
interpet "Read the prompt" quite literally and just read it.
I think it's better to change that to "Read and execute".
-->
review: fix review with gemini cli
Gemini does not take -- before prompt. the prompt we supply can not
be mistaken for a flag, so it is not needed by any agents.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
diff --git a/src/b4/review_tui/_review_app.py b/src/b4/review_tui/_review_app.py
index e1c988e..12eb146 100644
--- a/src/b4/review_tui/_review_app.py
+++ b/src/b4/review_tui/_review_app.py
@@ -1313,7 +1313,7 @@ class ReviewApp(App[None]):
self.notify(f'Agent prompt file not found: {agent_prompt}',
severity='error')
return
- cmdargs += ['--', f'Read the prompt from {prompt_path}']
+ cmdargs += [f'Read the prompt from {prompt_path}']
with self.suspend():
logger.info('Running review agent: %s', ' '.join(cmdargs))
^ permalink raw reply related [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-28 20:23 ` Michael S. Tsirkin
@ 2026-02-28 20:37 ` Konstantin Ryabitsev
2026-02-28 20:47 ` Michael S. Tsirkin
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-02-28 20:37 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: users, tools
On Sat, Feb 28, 2026 at 03:23:32PM -0500, Michael S. Tsirkin wrote:
> Gemini does not take -- before prompt. the prompt we supply can not
> be mistaken for a flag, so it is not needed by any agents.
Bah, that "--" was needed for Claude. /o\
I'll play around with it some more and try to find a solution that satisfies
most agents.
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-28 20:37 ` Konstantin Ryabitsev
@ 2026-02-28 20:47 ` Michael S. Tsirkin
2026-02-28 20:53 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Michael S. Tsirkin @ 2026-02-28 20:47 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
On Sat, Feb 28, 2026 at 03:37:30PM -0500, Konstantin Ryabitsev wrote:
> On Sat, Feb 28, 2026 at 03:23:32PM -0500, Michael S. Tsirkin wrote:
> > Gemini does not take -- before prompt. the prompt we supply can not
> > be mistaken for a flag, so it is not needed by any agents.
>
> Bah, that "--" was needed for Claude. /o\
>
> I'll play around with it some more and try to find a solution that satisfies
> most agents.
>
> --
> KR
at least for me, claude works without --. shrug.
$ claude "say hello"
▐▛███▜▌ Claude Code v2.1.63
▝▜█████▛▘ Opus 4.6 (1M context) · API Usage Billing
▘▘ ▝▝ /scm/b4
Welcome to Opus 4.6
❯ say hello
● Hello! How can I help you today?
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
❯
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
? for shortcuts 21722 tokens
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-28 20:47 ` Michael S. Tsirkin
@ 2026-02-28 20:53 ` Konstantin Ryabitsev
2026-02-28 21:04 ` Michael S. Tsirkin
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-02-28 20:53 UTC (permalink / raw)
To: Michael S. Tsirkin; +Cc: users, tools
On Sat, Feb 28, 2026 at 03:47:51PM -0500, Michael S. Tsirkin wrote:
> > Bah, that "--" was needed for Claude. /o\
>
> at least for me, claude works without --. shrug.
Yes, but that's without passing --allowedTools, which takes a really complex
format with a ton of spaces. I may deprecate that in lieu of shipping policy
files instead.
> $ claude "say hello"
>
> ? for shortcuts 21722 tokens
Gotta say, that's probably world's least efficient "hello world". ;)
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-28 20:53 ` Konstantin Ryabitsev
@ 2026-02-28 21:04 ` Michael S. Tsirkin
0 siblings, 0 replies; 69+ messages in thread
From: Michael S. Tsirkin @ 2026-02-28 21:04 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
On Sat, Feb 28, 2026 at 03:53:40PM -0500, Konstantin Ryabitsev wrote:
> On Sat, Feb 28, 2026 at 03:47:51PM -0500, Michael S. Tsirkin wrote:
> > > Bah, that "--" was needed for Claude. /o\
> >
> > at least for me, claude works without --. shrug.
>
> Yes, but that's without passing --allowedTools, which takes a really complex
> format with a ton of spaces. I may deprecate that in lieu of shipping policy
> files instead.
Since we are discussing this, I thought I'd share the following, written
a while ago with help from claude. I know I sleep better with this than
with running random stuff from the intertubes directly.
-->
review: add sandbox wrappers and docs for AI review agents
Add bwrap and firejail wrapper scripts that sandbox review agents
against prompt injection in untrusted patches. Document agent
configuration including a bunch of agents.
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
diff --git a/misc/bwrap-review-agent.sh b/misc/bwrap-review-agent.sh
new file mode 100755
index 0000000..460a21d
--- /dev/null
+++ b/misc/bwrap-review-agent.sh
@@ -0,0 +1,70 @@
+#!/bin/bash
+# bwrap-review-agent.sh -- bubblewrap sandbox for b4 review agents
+#
+# Full filesystem isolation: the entire filesystem is mounted
+# read-only, real HOME is replaced with a temporary copy containing
+# only agent auth config, and .git/b4-review/ is the sole writable
+# path. Agent binaries installed under HOME (npm, pip, cargo) are
+# re-exposed read-only.
+#
+# Requires: bubblewrap (bwrap)
+#
+# Usage -- set as your review-agent-command in git config:
+#
+# [b4]
+# review-agent-command = misc/bwrap-review-agent.sh cursor-agent --yolo
+# review-agent-prompt-path = .git/agent-reviewer.md
+
+set -euo pipefail
+
+mydir="$(cd "$(dirname "$0")" && pwd)"
+# shellcheck source=review-agent-sandbox-lib.sh
+. "$mydir/review-agent-sandbox-lib.sh"
+
+if ! command -v bwrap >/dev/null 2>&1; then
+ echo >&2 "bwrap-review-agent: bwrap not found, running agent without sandbox"
+ export HOME="$sandbox_home"
+ exec "$@"
+fi
+
+# Mount layout (later mounts override earlier ones):
+#
+# / read-only entire host filesystem
+# /dev read-write minimal private devtmpfs
+# /proc read-only procfs
+# /tmp read-write private tmpfs (empty, ephemeral)
+# $real_home read-write sandbox_home replaces real HOME;
+# ~/.ssh, ~/.gnupg, etc. are invisible
+# $real_home/.local/ read-only agent binaries (pip, pipx installs)
+# $real_home/.npm-*/ read-only agent binaries (npm global installs)
+# $real_home/.cargo/ read-only agent binaries (cargo installs)
+# $real_home/.nvm|... read-only node version managers
+# $topdir read-only the git repository
+# $review_dir read-write .git/b4-review/ -- review output
+#
+bwrap_args=(
+ --ro-bind / / # base: everything read-only
+ --dev /dev # private /dev
+ --proc /proc # procfs
+ --tmpfs /tmp # private /tmp
+ --bind "$sandbox_home" "${real_home}" # replace HOME with sandbox copy
+ --ro-bind "$topdir" "$topdir" # repo read-only
+ --bind "$review_dir" "$review_dir" # review output writable
+)
+
+# Re-expose directories under real HOME that contain agent binaries
+# and their libraries/runtimes (symlinks often point into lib dirs).
+# These are mounted read-only on top of the sandbox HOME.
+for d in \
+ .local/bin .local/share/cursor-agent \
+ .npm-global \
+ .cargo/bin \
+ .nvm .fnm .volta \
+; do
+ p="${real_home}/$d"
+ if [ -d "$p" ]; then
+ bwrap_args+=(--ro-bind "$p" "$p") # agent binary dir, read-only
+ fi
+done
+
+exec bwrap "${bwrap_args[@]}" -- "$@"
diff --git a/misc/firejail-review-agent.sh b/misc/firejail-review-agent.sh
new file mode 100755
index 0000000..aa8093f
--- /dev/null
+++ b/misc/firejail-review-agent.sh
@@ -0,0 +1,55 @@
+#!/bin/bash
+# firejail-review-agent.sh -- firejail sandbox for b4 review agents
+#
+# The repository and real HOME are mounted read-only. HOME env is
+# redirected to a temporary copy containing only agent auth config.
+# .git/b4-review/ is the sole writable path in the repository.
+#
+# Requires: firejail
+#
+# Note: firejail cannot fully hide the real HOME from the agent
+# (read-only != invisible). For stronger isolation, prefer
+# bwrap-review-agent.sh which replaces HOME entirely.
+#
+# Usage -- set as your review-agent-command in git config:
+#
+# [b4]
+# review-agent-command = misc/firejail-review-agent.sh cursor-agent --yolo
+# review-agent-prompt-path = .git/agent-reviewer.md
+
+set -euo pipefail
+
+mydir="$(cd "$(dirname "$0")" && pwd)"
+# shellcheck source=review-agent-sandbox-lib.sh
+. "$mydir/review-agent-sandbox-lib.sh"
+
+if ! command -v firejail >/dev/null 2>&1; then
+ echo >&2 "firejail-review-agent: firejail not found, running agent without sandbox"
+ export HOME="$sandbox_home"
+ exec "$@"
+fi
+
+# Mount layout:
+#
+# $real_home read-only real HOME (still visible but not writable;
+# ~/.ssh, ~/.gnupg are readable -- use bwrap
+# wrapper if this is a concern)
+# $topdir read-only the git repository
+# $review_dir read-write .git/b4-review/ -- review output
+# /tmp read-write private tmpfs (empty, ephemeral)
+# /dev read-write minimal private devtmpfs
+# HOME env sandbox_home agent looks here for config by default
+#
+# Capabilities: all dropped. No new privileges, no root.
+#
+export HOME="$sandbox_home"
+firejail --noprofile \
+ --read-only="${real_home}" \
+ --read-only="$topdir" \
+ --read-write="$review_dir" \
+ --private-tmp \
+ --private-dev \
+ --caps.drop=all \
+ --nonewprivs \
+ --noroot \
+ -- "$@"
diff --git a/misc/review-agent-sandbox-lib.sh b/misc/review-agent-sandbox-lib.sh
new file mode 100644
index 0000000..df588ff
--- /dev/null
+++ b/misc/review-agent-sandbox-lib.sh
@@ -0,0 +1,44 @@
+# review-agent-sandbox-lib.sh -- shared setup for sandbox wrappers
+#
+# Sourced by firejail-review-agent.sh and bwrap-review-agent.sh.
+# Not executable on its own.
+#
+# After sourcing, the following variables are set:
+# topdir -- repository top-level directory
+# review_dir -- .git/b4-review/ (writable review output)
+# real_home -- the user's real HOME
+# sandbox_home -- temporary HOME with only agent auth config
+#
+# A trap is registered to clean up sandbox_home on exit.
+
+topdir="$(git rev-parse --show-toplevel)"
+review_dir="$topdir/.git/b4-review"
+mkdir -p "$review_dir"
+real_home="${HOME}"
+
+# Create a temporary HOME containing only agent auth config.
+# The agent process sees this as $HOME, so ~/.ssh, ~/.gnupg, etc.
+# simply do not exist.
+mkdir -p "$review_dir/.sandbox"
+sandbox_home="$(mktemp -d --tmpdir="$review_dir/.sandbox")"
+trap 'rm -rf "$sandbox_home"' EXIT INT TERM
+
+# Copy top-level config files (auth, settings) for known agents.
+# Only files under 1 MB are copied -- large session logs and caches
+# are skipped since the agent only needs credentials.
+for d in \
+ .config/cursor .config/Cursor .cursor \
+ .config/claude .claude .local/share/claude .local/state/claude \
+ .codex \
+ .gemini .config/gemini \
+; do
+ src="${real_home}/$d"
+ if [ -d "$src" ]; then
+ mkdir -p "$sandbox_home/$d"
+ find "$src" -maxdepth 1 -type f -size -1024k \
+ -exec cp -a {} "$sandbox_home/$d/" \;
+ elif [ -e "$src" ] || [ -L "$src" ]; then
+ mkdir -p "$(dirname "$sandbox_home/$d")"
+ cp -a "$src" "$sandbox_home/$d"
+ fi
+done
^ permalink raw reply related [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-27 19:53 b4 review available in master Konstantin Ryabitsev
` (3 preceding siblings ...)
2026-02-28 18:31 ` Michael S. Tsirkin
@ 2026-03-02 9:30 ` Michael S. Tsirkin
2026-03-02 10:33 ` Michael S. Tsirkin
` (4 subsequent siblings)
9 siblings, 0 replies; 69+ messages in thread
From: Michael S. Tsirkin @ 2026-03-02 9:30 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
On Fri, Feb 27, 2026 at 02:53:07PM -0500, Konstantin Ryabitsev wrote:
> Hi, all:
>
> TLDR: "b4 review" is a terminal UI that hopes to streamline a lot of
> maintainer duties. A getting-started guide with screencasts is here:
> https://b4.docs.kernel.org/en/latest/reviewer/getting-started.html
>
> It's available from b4 master pending initial testing and will be generally
> available in b4-0.15.
OK I just tried the "update" flow, and maybe it worked, thanks!
Though I was confused for a while.
I had this:
Date Status Submitter Subject
2026-02-28 reviewing Vishwanath Seshagiri virtio_net: add page_pool support for buffer allocation
2025-10-13 new Michael S. Tsirkin b4: sparse-checkout related fixes
(... lots of empty lines ...)
─────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────────
Subject: [v8,0/1] virtio_net: add page_pool support for buffer allocation
From: Vishwanath Seshagiri <vishs@meta.com>
Sent: Sat, 28 Feb 2026 08:41:22 -0500
Change-ID: 20260228-virtio_net-add-page_pool-support-for-buffer-allocation-6273c5614ede
Link: https://patch.msgid.link/20260228134122.631580-1-vishs@meta.com
Revisions: v2, v3, v4, v5, v6, v7, v8, v9 (upgrade from v8 with [a]ction)
A/R/T: - / - / -
Branch: b4/review/20260228-virtio_net-add-page_pool-support-for-buffer-allocation-6273c5614ede
r review d range-diff a action ▏ u update l limit s shell q quit ? help ▏^p palette
So I was confused: I can see there is a v9 but why is it still showing me v8?
It took me a while until I noticed Revisions below and a while more
until I understood: this is intentional,
it means that yes it is v8 but v9 is available and if I press action
I can upgrade. Maybe putting a * near the current revision will help.
Maybe adding (upgrade to v9 with [a]ction) near v8 in the subject will
help.
Maybe showing "press a for [a]ction now to apply the upgrade" will help.
Maybe automatically triggering the correct action is the right thing?
I wonder why not.
"action" is also very generic. Maybe saying More [a]ctions in the menu will
better hint to the user it's like a ... and worth exploring.
Then I did "a" and I got a list of options none of which is "update"
or mentions v8 or v9. I guessed "rebase review branch" does it
and it seems to, but I have no idea why and what rebase
does it refer to:
┌───────────────────────────────────────────┐
│ │
│ Select action: │
│ Take (apply to branch) │
│ Rebase review branch │
│ Mark as waiting on new revision │
│ Abandon series │
│ Archive series │
│ │
└───────────────────────────────────────────┘
does it always update to latest revision? rename accordingly?
also actually saying what is rebased to what in the menu would have
helped.
Overall I was able to figure out things, and it worked!
The notes are here to maybe polish the UI even more.
Thanks!
--
MST
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-27 19:53 b4 review available in master Konstantin Ryabitsev
` (4 preceding siblings ...)
2026-03-02 9:30 ` Michael S. Tsirkin
@ 2026-03-02 10:33 ` Michael S. Tsirkin
2026-03-03 1:58 ` Junio C Hamano
` (3 subsequent siblings)
9 siblings, 0 replies; 69+ messages in thread
From: Michael S. Tsirkin @ 2026-03-02 10:33 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
On Fri, Feb 27, 2026 at 02:53:07PM -0500, Konstantin Ryabitsev wrote:
> Hi, all:
>
> TLDR: "b4 review" is a terminal UI that hopes to streamline a lot of
> maintainer duties. A getting-started guide with screencasts is here:
> https://b4.docs.kernel.org/en/latest/reviewer/getting-started.html
I tried:
b4 review track 20260302090413.86471-1-xuanzhuo@linux.alibaba.com
but it is unhappy - no commit there.
Add an option to specify commit manually?
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-02-27 19:53 b4 review available in master Konstantin Ryabitsev
` (5 preceding siblings ...)
2026-03-02 10:33 ` Michael S. Tsirkin
@ 2026-03-03 1:58 ` Junio C Hamano
2026-03-03 4:26 ` Konstantin Ryabitsev
2026-03-04 20:56 ` range-diff hangs Marc Kleine-Budde
` (2 subsequent siblings)
9 siblings, 1 reply; 69+ messages in thread
From: Junio C Hamano @ 2026-03-03 1:58 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
I hate to complain about trivialities, but the coloring scheme was
too noisy and with weaker color perception than normal folks, I
couldn't really see many of the UI elements in "b4 review tui".
Is there a monochrome option that is usable by color challenged
people?
Thanks.
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-03-03 1:58 ` Junio C Hamano
@ 2026-03-03 4:26 ` Konstantin Ryabitsev
2026-03-03 11:20 ` Matthieu Baerts
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-03 4:26 UTC (permalink / raw)
To: Junio C Hamano; +Cc: users, tools
On Mon, 02 Mar 2026, Junio C Hamano wrote:
> I hate to complain about trivialities, but the coloring scheme was
> too noisy and with weaker color perception than normal folks, I
> couldn't really see many of the UI elements in "b4 review tui".
Junio:
Thanks for giving b4 review a try -- and this is definitely not a triviality.
A lot of it is "rapidly prototyped" with colours that made sense on my
dark-mode terminal using textual's default pallette and I've not tried it with
other colour schemes.
> Is there a monochrome option that is usable by color challenged
> people?
There is now. The review TUI is built on top of Textual, which ships a
built-in 16-colour ANSI theme that defers all colour choices to the terminal
emulator. You can enable it by setting:
export TEXTUAL_THEME=textual-ansi
With this, the TUI will use your terminal's own palette instead of hardcoded
RGB values, so it should work well with any colour scheme you've configured
for your accessibility needs -- whether that's high-contrast, monochrome, or a
palette tuned for colour vision deficiency.
You'll still find some colours here and there -- please let me know if they
are a distraction and I'll think of better ways to make that work better in
monochrome modes.
You'll need the latest b4 master for this.
Best wishes,
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-03-03 4:26 ` Konstantin Ryabitsev
@ 2026-03-03 11:20 ` Matthieu Baerts
0 siblings, 0 replies; 69+ messages in thread
From: Matthieu Baerts @ 2026-03-03 11:20 UTC (permalink / raw)
To: Konstantin Ryabitsev, Junio C Hamano; +Cc: users, tools
Hi Konstantin, Junio,
Thank you for this new feature! And for maintaining git :)
On 03/03/2026 05:26, Konstantin Ryabitsev wrote:
> On Mon, 02 Mar 2026, Junio C Hamano wrote:
>> I hate to complain about trivialities, but the coloring scheme was
>> too noisy and with weaker color perception than normal folks, I
>> couldn't really see many of the UI elements in "b4 review tui".
>
> Junio:
>
> Thanks for giving b4 review a try -- and this is definitely not a triviality.
> A lot of it is "rapidly prototyped" with colours that made sense on my
> dark-mode terminal using textual's default pallette and I've not tried it with
> other colour schemes.
>
>> Is there a monochrome option that is usable by color challenged
>> people?
>
> There is now. The review TUI is built on top of Textual, which ships a
> built-in 16-colour ANSI theme that defers all colour choices to the terminal
> emulator. You can enable it by setting:
>
> export TEXTUAL_THEME=textual-ansi
Just in case, like many other TUI tools and libs, Textual supports
'NO_COLOR' env var [1]. So an alternative is to use:
NO_COLOR=1 b4 review tui
[1] https://no-color.org
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
^ permalink raw reply [flat|nested] 69+ messages in thread
* range-diff hangs
2026-02-27 19:53 b4 review available in master Konstantin Ryabitsev
` (6 preceding siblings ...)
2026-03-03 1:58 ` Junio C Hamano
@ 2026-03-04 20:56 ` Marc Kleine-Budde
2026-03-14 4:20 ` Konstantin Ryabitsev
2026-03-16 23:28 ` b4 review available in master Jonathan Corbet
2026-03-20 16:53 ` Louis Chauvet
9 siblings, 1 reply; 69+ messages in thread
From: Marc Kleine-Budde @ 2026-03-04 20:56 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 1524 bytes --]
On 27.02.2026 14:53:07, Konstantin Ryabitsev wrote:
> TLDR: "b4 review" is a terminal UI that hopes to streamline a lot of
> maintainer duties. A getting-started guide with screencasts is here:
> https://b4.docs.kernel.org/en/latest/reviewer/getting-started.html
I'm using b4 v0.12.0-419-g18f8fdb27a22 and a range diff "hangs":
| $ b4 review tui
| Fetching v2 from lore...
| Looking up https://lore.kernel.org/all/20260219-bcm_spin_lock_init-v2-1-694352bfca62@hartkopp.net/
| Grabbing thread from lore.kernel.org/all/20260219-bcm_spin_lock_init-v2-1-694352bfca62@hartkopp.net/t.mbox.gz
| Checking for newer revisions
| Added from v3: 4 patches
| Checking for older revisions
| Preparing fake-am range for v2...
| Preparing fake-am for v2: can: bcm: add/fix locking for config updates at runtime
| range: d9c253b4e17a..88c829c06921
| Running range-diff...
The relevant part of "ps auxwwf"
| python3 local/git/b4/src/b4/command.py review tui
| \_ /usr/bin/perl local/bin/diff-highlight | less -RFX -x1,9 pengutronix/socketcan/linux-can/.b4-pager66hzgdqv/range-diff.txt
The diff-highlight and the less is configured in my "~/.gitconfig"
| $ git config get core.pager
| diff-highlight | less -RFX -x1,9
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: range-diff hangs
2026-03-04 20:56 ` range-diff hangs Marc Kleine-Budde
@ 2026-03-14 4:20 ` Konstantin Ryabitsev
2026-03-14 9:27 ` Marc Kleine-Budde
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-14 4:20 UTC (permalink / raw)
To: Marc Kleine-Budde; +Cc: users, tools
On Wed, Mar 04, 2026 at 09:56:35PM +0100, Marc Kleine-Budde wrote:
> On 27.02.2026 14:53:07, Konstantin Ryabitsev wrote:
> > TLDR: "b4 review" is a terminal UI that hopes to streamline a lot of
> > maintainer duties. A getting-started guide with screencasts is here:
> > https://b4.docs.kernel.org/en/latest/reviewer/getting-started.html
>
> I'm using b4 v0.12.0-419-g18f8fdb27a22 and a range diff "hangs":
> | $ git config get core.pager
> | diff-highlight | less -RFX -x1,9
I believe we should be handing the | in the core.pager config now, but please
test out the latest master to confirm.
Regards,
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: range-diff hangs
2026-03-14 4:20 ` Konstantin Ryabitsev
@ 2026-03-14 9:27 ` Marc Kleine-Budde
0 siblings, 0 replies; 69+ messages in thread
From: Marc Kleine-Budde @ 2026-03-14 9:27 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
[-- Attachment #1: Type: text/plain, Size: 1015 bytes --]
On 14.03.2026 00:20:47, Konstantin Ryabitsev wrote:
> On Wed, Mar 04, 2026 at 09:56:35PM +0100, Marc Kleine-Budde wrote:
> > On 27.02.2026 14:53:07, Konstantin Ryabitsev wrote:
> > > TLDR: "b4 review" is a terminal UI that hopes to streamline a lot of
> > > maintainer duties. A getting-started guide with screencasts is here:
> > > https://b4.docs.kernel.org/en/latest/reviewer/getting-started.html
> >
> > I'm using b4 v0.12.0-419-g18f8fdb27a22 and a range diff "hangs":
>
> > | $ git config get core.pager
> > | diff-highlight | less -RFX -x1,9
>
> I believe we should be handing the | in the core.pager config now, but please
> test out the latest master to confirm.
Thanks, latest master fixes the problem.
regards,
Marc
--
Pengutronix e.K. | Marc Kleine-Budde |
Embedded Linux | https://www.pengutronix.de |
Vertretung Nürnberg | Phone: +49-5121-206917-129 |
Amtsgericht Hildesheim, HRA 2686 | Fax: +49-5121-206917-9 |
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-27 19:53 b4 review available in master Konstantin Ryabitsev
` (7 preceding siblings ...)
2026-03-04 20:56 ` range-diff hangs Marc Kleine-Budde
@ 2026-03-16 23:28 ` Jonathan Corbet
2026-03-16 23:41 ` Jonathan Corbet
` (2 more replies)
2026-03-20 16:53 ` Louis Chauvet
9 siblings, 3 replies; 69+ messages in thread
From: Jonathan Corbet @ 2026-03-16 23:28 UTC (permalink / raw)
To: Konstantin Ryabitsev, users, tools
Konstantin Ryabitsev <mricon@kernel.org> writes:
> Hi, all:
>
> TLDR: "b4 review" is a terminal UI that hopes to streamline a lot of
> maintainer duties. A getting-started guide with screencasts is here:
> https://b4.docs.kernel.org/en/latest/reviewer/getting-started.html
So I've been playing with this a bit - fun stuff! A few initial
comments.
Let me be the N+1th to complain about the color scheme. Dark mode just
isn't my thing... and whoever designed "textual-light" has drunk
copious amounts of the low-contrast koolaid. I see some of the other
suggestions in the thread, haven't tried them yet.
I used it to bring in a series and comment on a patch. Nice
but... there doesn't seem to be a way to comment on the changelog?
That's an important part of the patch as a whole and I, at least, often
have things to say there.
Having descended onto a 30-part Mauro marathon, I can't a way to get
back to the main screen showing all of the tracked patches...? "q" just
drops me out entirely.
When can we have it as an Emacs mode? :)
Thanks for all of this,
jon
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-03-16 23:28 ` b4 review available in master Jonathan Corbet
@ 2026-03-16 23:41 ` Jonathan Corbet
2026-03-17 0:15 ` Konstantin Ryabitsev
2026-03-17 0:12 ` Konstantin Ryabitsev
2026-03-18 13:43 ` Johannes Thumshirn
2 siblings, 1 reply; 69+ messages in thread
From: Jonathan Corbet @ 2026-03-16 23:41 UTC (permalink / raw)
To: Konstantin Ryabitsev, users, tools
Jonathan Corbet <corbet@lwn.net> writes:
> Konstantin Ryabitsev <mricon@kernel.org> writes:
>
>> Hi, all:
>>
>> TLDR: "b4 review" is a terminal UI that hopes to streamline a lot of
>> maintainer duties. A getting-started guide with screencasts is here:
>> https://b4.docs.kernel.org/en/latest/reviewer/getting-started.html
>
> So I've been playing with this a bit - fun stuff! A few initial
> comments.
One more... somehow it totally mangled the placement of the comments in
the first review I did with it. See
https://lore.kernel.org/all/177370220974.1754131.9642805524574261129.b4-review@b4/
...and the followup with the intended placement restored.
Thanks,
jon
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-16 23:41 ` Jonathan Corbet
@ 2026-03-17 0:15 ` Konstantin Ryabitsev
2026-03-17 14:11 ` Jonathan Corbet
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-17 0:15 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: users, tools
On Mon, Mar 16, 2026 at 05:41:47PM -0600, Jonathan Corbet wrote:
> > Konstantin Ryabitsev <mricon@kernel.org> writes:
> >
> >> Hi, all:
> >>
> >> TLDR: "b4 review" is a terminal UI that hopes to streamline a lot of
> >> maintainer duties. A getting-started guide with screencasts is here:
> >> https://b4.docs.kernel.org/en/latest/reviewer/getting-started.html
> >
> > So I've been playing with this a bit - fun stuff! A few initial
> > comments.
>
> One more... somehow it totally mangled the placement of the comments in
> the first review I did with it. See
>
> https://lore.kernel.org/all/177370220974.1754131.9642805524574261129.b4-review@b4/
>
> ...and the followup with the intended placement restored.
Failure is always an option! :)
Thanks for both of these. Can you possibly share the review branch so I can
see the tracking commit from there?
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-17 0:15 ` Konstantin Ryabitsev
@ 2026-03-17 14:11 ` Jonathan Corbet
2026-03-17 14:23 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Jonathan Corbet @ 2026-03-17 14:11 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
Konstantin Ryabitsev <mricon@kernel.org> writes:
> On Mon, Mar 16, 2026 at 05:41:47PM -0600, Jonathan Corbet wrote:
>> > Konstantin Ryabitsev <mricon@kernel.org> writes:
>> >
>> >> Hi, all:
>> >>
>> >> TLDR: "b4 review" is a terminal UI that hopes to streamline a lot of
>> >> maintainer duties. A getting-started guide with screencasts is here:
>> >> https://b4.docs.kernel.org/en/latest/reviewer/getting-started.html
>> >
>> > So I've been playing with this a bit - fun stuff! A few initial
>> > comments.
>>
>> One more... somehow it totally mangled the placement of the comments in
>> the first review I did with it. See
>>
>> https://lore.kernel.org/all/177370220974.1754131.9642805524574261129.b4-review@b4/
>>
>> ...and the followup with the intended placement restored.
>
> Failure is always an option! :)
>
> Thanks for both of these. Can you possibly share the review branch so I can
> see the tracking commit from there?
If I've done everything right, you'll find what you're looking for at
git://git.lwn.net/linux.git b4/review/20260312-kernel-doc-use-a-c-lexical-tokenizer-for-transforms-0e0e9e143327
Thanks,
jon
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-17 14:11 ` Jonathan Corbet
@ 2026-03-17 14:23 ` Konstantin Ryabitsev
2026-03-17 20:30 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-17 14:23 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: users, tools
On Tue, Mar 17, 2026 at 08:11:26AM -0600, Jonathan Corbet wrote:
> > Thanks for both of these. Can you possibly share the review branch so I can
> > see the tracking commit from there?
>
> If I've done everything right, you'll find what you're looking for at
>
> git://git.lwn.net/linux.git b4/review/20260312-kernel-doc-use-a-c-lexical-tokenizer-for-transforms-0e0e9e143327
Yep, found it. *sigh*
Unfortunately, it's not an easy fix. I think I'll have to
abandon how we currently do inline comments -- the diff format is just too
fragile to try to intersperse it with comments. I think I'm just going to load
the diff using standard "> " quoting instead of trying to be creative with >>>
<<< markers. Maintainers are more used to seeing that anyway.
I'll let you know when the new code is ready for testing.
Thanks!
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-17 14:23 ` Konstantin Ryabitsev
@ 2026-03-17 20:30 ` Konstantin Ryabitsev
2026-03-17 21:46 ` Jonathan Corbet
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-17 20:30 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: users, tools
On Tue, Mar 17, 2026 at 10:23:33AM -0400, Konstantin Ryabitsev wrote:
> > If I've done everything right, you'll find what you're looking for at
> >
> > git://git.lwn.net/linux.git b4/review/20260312-kernel-doc-use-a-c-lexical-tokenizer-for-transforms-0e0e9e143327
>
> Yep, found it. *sigh*
>
> Unfortunately, it's not an easy fix. I think I'll have to
> abandon how we currently do inline comments -- the diff format is just too
> fragile to try to intersperse it with comments. I think I'm just going to load
> the diff using standard "> " quoting instead of trying to be creative with >>>
> <<< markers. Maintainers are more used to seeing that anyway.
>
> I'll let you know when the new code is ready for testing.
Okay, a fairly big change landed in master that completely does away with "C"
and inline comments. You now just hit "r" to reply and add your comments just
like you would to any other message -- benefiting from your existing mastery
of your preferred editor.
We also ship vim and emacs syntax files in misc/ -- they make it easier to
reply to diffs than regular email syntax styling.
Please try it out again. Your old bugs should be gone and replaced by whole
new bugs!
Best regards,
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-17 20:30 ` Konstantin Ryabitsev
@ 2026-03-17 21:46 ` Jonathan Corbet
2026-03-17 22:39 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Jonathan Corbet @ 2026-03-17 21:46 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
Konstantin Ryabitsev <mricon@kernel.org> writes:
> On Tue, Mar 17, 2026 at 10:23:33AM -0400, Konstantin Ryabitsev wrote:
>> > If I've done everything right, you'll find what you're looking for at
>> >
>> > git://git.lwn.net/linux.git b4/review/20260312-kernel-doc-use-a-c-lexical-tokenizer-for-transforms-0e0e9e143327
>>
>> Yep, found it. *sigh*
>>
>> Unfortunately, it's not an easy fix. I think I'll have to
>> abandon how we currently do inline comments -- the diff format is just too
>> fragile to try to intersperse it with comments. I think I'm just going to load
>> the diff using standard "> " quoting instead of trying to be creative with >>>
>> <<< markers. Maintainers are more used to seeing that anyway.
>>
>> I'll let you know when the new code is ready for testing.
>
> Okay, a fairly big change landed in master that completely does away with "C"
> and inline comments. You now just hit "r" to reply and add your comments just
> like you would to any other message -- benefiting from your existing mastery
> of your preferred editor.
>
> We also ship vim and emacs syntax files in misc/ -- they make it easier to
> reply to diffs than regular email syntax styling.
>
> Please try it out again. Your old bugs should be gone and replaced by whole
> new bugs!
So it's still behaving a bit strangely, at least that's how it seems to
me. I tried adding comments to a patch, doing my usual trick of
trimming out stuff that is being skipped over. Quitting that, then
asking for email mode showed me a blank message, which is rather more
terse than I usually am.
So I went and did it again, simply inserting a comment in the patch
without removing anything, but it still shows up in the wrong place. I
guess it just doesn't like me...?
(I can push another branch on request).
jon
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-17 21:46 ` Jonathan Corbet
@ 2026-03-17 22:39 ` Konstantin Ryabitsev
2026-03-17 23:37 ` Konstantin Ryabitsev
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-17 22:39 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: users, tools
On Tue, Mar 17, 2026 at 03:46:05PM -0600, Jonathan Corbet wrote:
> > Please try it out again. Your old bugs should be gone and replaced by whole
> > new bugs!
>
> So it's still behaving a bit strangely, at least that's how it seems to
> me. I tried adding comments to a patch, doing my usual trick of
> trimming out stuff that is being skipped over.
You don't need to do that. Just find the stuff you want commenting on. We'll
take care of the rest! :)
But, I'm able to reproduce what you're seeing -- trimming content doesn't do
the right thing. Let me see if I can make this a lot more relaxed.
(But, for now, just add your comments without trimming anything -- the
resulting email will be trimmed automatically).
Regards,
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-17 22:39 ` Konstantin Ryabitsev
@ 2026-03-17 23:37 ` Konstantin Ryabitsev
2026-03-18 7:56 ` Geert Uytterhoeven
2026-03-18 16:47 ` Jonathan Corbet
0 siblings, 2 replies; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-17 23:37 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: users, tools
On Tue, Mar 17, 2026 at 06:39:54PM -0400, Konstantin Ryabitsev wrote:
> > So it's still behaving a bit strangely, at least that's how it seems to
> > me. I tried adding comments to a patch, doing my usual trick of
> > trimming out stuff that is being skipped over.
>
> You don't need to do that. Just find the stuff you want commenting on. We'll
> take care of the rest! :)
>
> But, I'm able to reproduce what you're seeing -- trimming content doesn't do
> the right thing. Let me see if I can make this a lot more relaxed.
The current master should be a lot more forgiving when parsing your comments.
However, I am now curious to hear your feedback. The idea with the review app
is that you are not really sending an email, you're adding comments to code
and adding review trailers. The app will render and send that out for you,
trimming and formatting things nicely so you don't have to do it on your own
(I've noticed that many maintainers don't bother anyway -- there are emails
with a terse comment at the top and then a huge quoted bottom that is only
followed by their sig.
Is this rife for creating confusion? Will maintainers be fighting with this
and growing grumpy because it's not exactly like their email client?
Thanks for trying it out!
Regards,
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-17 23:37 ` Konstantin Ryabitsev
@ 2026-03-18 7:56 ` Geert Uytterhoeven
2026-03-18 13:00 ` Mark Brown
2026-03-18 13:26 ` Konstantin Ryabitsev
2026-03-18 16:47 ` Jonathan Corbet
1 sibling, 2 replies; 69+ messages in thread
From: Geert Uytterhoeven @ 2026-03-18 7:56 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: Jonathan Corbet, users, tools
Hi Konstantin,
On Wed, 18 Mar 2026 at 00:37, Konstantin Ryabitsev <mricon@kernel.org> wrote:
> On Tue, Mar 17, 2026 at 06:39:54PM -0400, Konstantin Ryabitsev wrote:
> > > So it's still behaving a bit strangely, at least that's how it seems to
> > > me. I tried adding comments to a patch, doing my usual trick of
> > > trimming out stuff that is being skipped over.
> >
> > You don't need to do that. Just find the stuff you want commenting on. We'll
> > take care of the rest! :)
> >
> > But, I'm able to reproduce what you're seeing -- trimming content doesn't do
> > the right thing. Let me see if I can make this a lot more relaxed.
>
> The current master should be a lot more forgiving when parsing your comments.
>
> However, I am now curious to hear your feedback. The idea with the review app
> is that you are not really sending an email, you're adding comments to code
> and adding review trailers. The app will render and send that out for you,
> trimming and formatting things nicely so you don't have to do it on your own
I haven't tried b4 review yet, but how do you decide what (not) to trim?
It is not uncommon to explicitly not trim some parts, as they are important
to understand the comment on a seemingly unrelated part.
> (I've noticed that many maintainers don't bother anyway -- there are emails
> with a terse comment at the top and then a huge quoted bottom that is only
> followed by their sig.
IMHO this is very bad behavior: not trimming replies forces everyone
to skim the remainder for more comments.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-03-18 7:56 ` Geert Uytterhoeven
@ 2026-03-18 13:00 ` Mark Brown
2026-03-18 13:26 ` Konstantin Ryabitsev
1 sibling, 0 replies; 69+ messages in thread
From: Mark Brown @ 2026-03-18 13:00 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Konstantin Ryabitsev, Jonathan Corbet, users, tools
[-- Attachment #1: Type: text/plain, Size: 1181 bytes --]
On Wed, Mar 18, 2026 at 08:56:56AM +0100, Geert Uytterhoeven wrote:
> On Wed, 18 Mar 2026 at 00:37, Konstantin Ryabitsev <mricon@kernel.org> wrote:
> > However, I am now curious to hear your feedback. The idea with the review app
> > is that you are not really sending an email, you're adding comments to code
> > and adding review trailers. The app will render and send that out for you,
> > trimming and formatting things nicely so you don't have to do it on your own
> I haven't tried b4 review yet, but how do you decide what (not) to trim?
> It is not uncommon to explicitly not trim some parts, as they are important
> to understand the comment on a seemingly unrelated part.
I also find that it's frequently useful to trim things while writing a
reply to a patch to make the message more managable and show what bits
still need commenting on.
> > (I've noticed that many maintainers don't bother anyway -- there are emails
> > with a terse comment at the top and then a huge quoted bottom that is only
> > followed by their sig.
> IMHO this is very bad behavior: not trimming replies forces everyone
> to skim the remainder for more comments.
It's extremely annoying.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-18 7:56 ` Geert Uytterhoeven
2026-03-18 13:00 ` Mark Brown
@ 2026-03-18 13:26 ` Konstantin Ryabitsev
1 sibling, 0 replies; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-18 13:26 UTC (permalink / raw)
To: Geert Uytterhoeven; +Cc: Jonathan Corbet, users, tools
On Wed, Mar 18, 2026 at 08:56:56AM +0100, Geert Uytterhoeven wrote:
> > However, I am now curious to hear your feedback. The idea with the review app
> > is that you are not really sending an email, you're adding comments to code
> > and adding review trailers. The app will render and send that out for you,
> > trimming and formatting things nicely so you don't have to do it on your own
>
> I haven't tried b4 review yet, but how do you decide what (not) to trim?
> It is not uncommon to explicitly not trim some parts, as they are important
> to understand the comment on a seemingly unrelated part.
We are obviously opinionated, but so is git, which by default gives 3 lines of
before/after context. Our logic is:
- always leave file and hunk headers intact
- give at least 6 lines of context above your content (double git's)
- check the remaining lines between the previous comment or the hunk start
(@@ .. @@) and trim if more than 10 lines, replacing with
[... skip NN lines ...]
If you're commenting on some code that requires a lot of context, it's easy to
do:
Let's look at this chunk:
> context
> -bar
> +foo
Using foo here is not correct.
> context
> context
>-baz
>+quux
etc
But, honestly, I think we're giving lots of context for the original
maintainer to understand what the comment is about, and since the file/hunk
headers are there, they can always find the code in question.
> > (I've noticed that many maintainers don't bother anyway -- there are emails
> > with a terse comment at the top and then a huge quoted bottom that is only
> > followed by their sig.
>
> IMHO this is very bad behavior: not trimming replies forces everyone
> to skim the remainder for more comments.
I wholeheartedly agree, but pointing and shaming probably isn't the right
course of action.
Regards,
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-17 23:37 ` Konstantin Ryabitsev
2026-03-18 7:56 ` Geert Uytterhoeven
@ 2026-03-18 16:47 ` Jonathan Corbet
2026-03-18 18:31 ` Laurent Pinchart
2026-03-18 19:22 ` Konstantin Ryabitsev
1 sibling, 2 replies; 69+ messages in thread
From: Jonathan Corbet @ 2026-03-18 16:47 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: users, tools
Konstantin Ryabitsev <mricon@kernel.org> writes:
> On Tue, Mar 17, 2026 at 06:39:54PM -0400, Konstantin Ryabitsev wrote:
>> > So it's still behaving a bit strangely, at least that's how it seems to
>> > me. I tried adding comments to a patch, doing my usual trick of
>> > trimming out stuff that is being skipped over.
>>
>> You don't need to do that. Just find the stuff you want commenting on. We'll
>> take care of the rest! :)
>>
>> But, I'm able to reproduce what you're seeing -- trimming content doesn't do
>> the right thing. Let me see if I can make this a lot more relaxed.
>
> The current master should be a lot more forgiving when parsing your comments.
>
> However, I am now curious to hear your feedback. The idea with the review app
> is that you are not really sending an email, you're adding comments to code
> and adding review trailers. The app will render and send that out for you,
> trimming and formatting things nicely so you don't have to do it on your own
> (I've noticed that many maintainers don't bother anyway -- there are emails
> with a terse comment at the top and then a huge quoted bottom that is only
> followed by their sig.
This is just me, of course ... and I was very much experimenting with it
as an alternative to my usual "do it in email" approach. So one can
definitely argue that I was simply holding it wrong.
If, though, the intent is that one inserts comments and leaves the patch
text alone, I think that the interface *really* needs to not let you
mess with the stuff you're not supposed to change. If one were to
implement this as <obnoxious>an Emacs mode</obnoxious>, that sort of
constraint could be implemented fairly easily. If you're throwing
somebody into an arbitrary editor, it's going to be rather harder.
I tend to be pretty careful about which text I trim when composing
replies. It's pretty helpful to, for example, be able to do something
like:
> something from the patch
Here you're doing X...
[...]
> something rather further down
Here instead it's expecting Y, WTF?
...so I would be less than fully pleased with an interface that prevents
that.
But perhaps I'm a dinosaur who just isn't using the tool correctly.
> Is this rife for creating confusion? Will maintainers be fighting with this
> and growing grumpy because it's not exactly like their email client?
Some surely will, see above. But that doesn't necessarily mean you're
not creating a better workflow in the long run. I think we need to play
with a lot of ideas, and I'm really glad you're doing that.
Thanks,
jon
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-18 16:47 ` Jonathan Corbet
@ 2026-03-18 18:31 ` Laurent Pinchart
2026-03-18 19:22 ` Konstantin Ryabitsev
1 sibling, 0 replies; 69+ messages in thread
From: Laurent Pinchart @ 2026-03-18 18:31 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: Konstantin Ryabitsev, users, tools
On Wed, Mar 18, 2026 at 10:47:07AM -0600, Jonathan Corbet wrote:
> Konstantin Ryabitsev <mricon@kernel.org> writes:
>
> > On Tue, Mar 17, 2026 at 06:39:54PM -0400, Konstantin Ryabitsev wrote:
> >> > So it's still behaving a bit strangely, at least that's how it seems to
> >> > me. I tried adding comments to a patch, doing my usual trick of
> >> > trimming out stuff that is being skipped over.
> >>
> >> You don't need to do that. Just find the stuff you want commenting on. We'll
> >> take care of the rest! :)
> >>
> >> But, I'm able to reproduce what you're seeing -- trimming content doesn't do
> >> the right thing. Let me see if I can make this a lot more relaxed.
> >
> > The current master should be a lot more forgiving when parsing your comments.
> >
> > However, I am now curious to hear your feedback. The idea with the review app
> > is that you are not really sending an email, you're adding comments to code
> > and adding review trailers. The app will render and send that out for you,
> > trimming and formatting things nicely so you don't have to do it on your own
> > (I've noticed that many maintainers don't bother anyway -- there are emails
> > with a terse comment at the top and then a huge quoted bottom that is only
> > followed by their sig.
What to trim and what not to trim is a question that will likely never
receive a single answer. I find it (slightly) annoying when people start
a conversation about a patch design in the text of the cover letter and
leave 1000 lines of diffstat below, but I find it more frustrating when
someone trims a patch agressively in their review and I end up not being
able to comment in my reply to both their comments and related parts of
the patch that they trimmed out.
The obvious answer is "trim exactly the way I would". I have had very
little success preaching that so far.
> This is just me, of course ... and I was very much experimenting with it
> as an alternative to my usual "do it in email" approach. So one can
> definitely argue that I was simply holding it wrong.
>
> If, though, the intent is that one inserts comments and leaves the patch
> text alone, I think that the interface *really* needs to not let you
> mess with the stuff you're not supposed to change. If one were to
> implement this as <obnoxious>an Emacs mode</obnoxious>, that sort of
> constraint could be implemented fairly easily. If you're throwing
> somebody into an arbitrary editor, it's going to be rather harder.
>
> I tend to be pretty careful about which text I trim when composing
> replies. It's pretty helpful to, for example, be able to do something
> like:
>
> > something from the patch
>
> Here you're doing X...
>
> [...]
> > something rather further down
>
> Here instead it's expecting Y, WTF?
>
> ...so I would be less than fully pleased with an interface that prevents
> that.
That's a style I sometimes use, usually not in the first e-mail reply,
but quite often when the mail thread is 10 replies deep and it becomes
clear large areas of the original patch are just not relevant to the
discussion any more.
> But perhaps I'm a dinosaur who just isn't using the tool correctly.
>
> > Is this rife for creating confusion? Will maintainers be fighting with this
> > and growing grumpy because it's not exactly like their email client?
>
> Some surely will, see above. But that doesn't necessarily mean you're
> not creating a better workflow in the long run. I think we need to play
> with a lot of ideas, and I'm really glad you're doing that.
--
Regards,
Laurent Pinchart
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-18 16:47 ` Jonathan Corbet
2026-03-18 18:31 ` Laurent Pinchart
@ 2026-03-18 19:22 ` Konstantin Ryabitsev
1 sibling, 0 replies; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-18 19:22 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: users, tools
On Wed, Mar 18, 2026 at 10:47:07AM -0600, Jonathan Corbet wrote:
> > However, I am now curious to hear your feedback. The idea with the review app
> > is that you are not really sending an email, you're adding comments to code
> > and adding review trailers. The app will render and send that out for you,
> > trimming and formatting things nicely so you don't have to do it on your own
> > (I've noticed that many maintainers don't bother anyway -- there are emails
> > with a terse comment at the top and then a huge quoted bottom that is only
> > followed by their sig.
>
> This is just me, of course ... and I was very much experimenting with it
> as an alternative to my usual "do it in email" approach. So one can
> definitely argue that I was simply holding it wrong.
I think it will help to look at not as an alternative to "do it in email" but
"do it alongside email." Email works great for person-to-person communication,
in-depth philosophical discussions, etc. But these are all fun and exciting
things about participating in Linux development already. What b4 review aims
to address is the "dull and boring" parts of participating in Linux
development:
- look through all diffs
- look at CI results
- drop to shell, poke around, run a local test
- send a quick Acked-by/Reviewed-by
- park it until you get a new revision
or
- take it in
- send a thanks
- archive it
> If, though, the intent is that one inserts comments and leaves the patch
> text alone, I think that the interface *really* needs to not let you
> mess with the stuff you're not supposed to change.
I went down that path. You could at some point insert comments via textual
pop-ups, but I hated it, because it's Not Like My Editor (also, see below).
> If one were to
> implement this as <obnoxious>an Emacs mode</obnoxious>, that sort of
> constraint could be implemented fairly easily. If you're throwing
> somebody into an arbitrary editor, it's going to be rather harder.
I've debated this with myself for so long! The reason the arbitrary editor won
is because it's so much easier to do powerful operations in it, including
things like: copy this original block of code, paste it and reformat it / edit
it to show how it should be done, add another buffer, edit the code there,
make sure it compiles, return to the original message, paste working code,
etc. All of this was dramatically more awkward when NOT done in your usual
favourite editor.
It only requires not looking at it as "sending an email" -- you're commenting
on code, don't treat this as the final email.
> I tend to be pretty careful about which text I trim when composing
> replies. It's pretty helpful to, for example, be able to do something
> like:
>
> > something from the patch
>
> Here you're doing X...
>
> [...]
> > something rather further down
>
> Here instead it's expecting Y, WTF?
>
> ...so I would be less than fully pleased with an interface that prevents
> that.
Noted! Again, this is supposed to work alongside your email client, so if you
find yourself constrained by the code review framework, you can always yank
that thread into your inbox (or reply from the lite email viewer that b4
review also provides).
> But perhaps I'm a dinosaur who just isn't using the tool correctly.
It *is* a bit of a challenge with the "users" crowd because everyone here is
very much already comfortable with their workflow. ;) Once 0.15 is out, I'm
hoping I get feedback from people who are not already well-established in
their ways (who, on the other hand, may find it awkward because it's "not like
Github").
> > Is this rife for creating confusion? Will maintainers be fighting with this
> > and growing grumpy because it's not exactly like their email client?
>
> Some surely will, see above. But that doesn't necessarily mean you're
> not creating a better workflow in the long run. I think we need to play
> with a lot of ideas, and I'm really glad you're doing that.
Great, thanks for the feedback!
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-16 23:28 ` b4 review available in master Jonathan Corbet
2026-03-16 23:41 ` Jonathan Corbet
@ 2026-03-17 0:12 ` Konstantin Ryabitsev
2026-03-18 13:43 ` Johannes Thumshirn
2 siblings, 0 replies; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-17 0:12 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: users, tools
On Mon, Mar 16, 2026 at 05:28:02PM -0600, Jonathan Corbet wrote:
> So I've been playing with this a bit - fun stuff! A few initial
> comments.
>
> Let me be the N+1th to complain about the color scheme.
No, you're the 2nd, actually. :)
> Dark mode just isn't my thing... and whoever designed "textual-light" has drunk
> copious amounts of the low-contrast koolaid.
You can see a bunch of them if you press ctrl+p to bring up the pallette and
select "Theme". If you find the one you like, you can set that via
"TEXTUAL_THEME" env var.
> I used it to bring in a series and comment on a patch. Nice
> but... there doesn't seem to be a way to comment on the changelog?
Just pressing "r" to reply should bring up the usual reply with everything
quoted, unless we're actually stripping the basement there. The recommended
workflow is to add your inline comments first and then press "r" -- this
*should* load up the commit message with your inline comments. Or you can jump
to "r" directly and work with it as with any email client.
If there are parts missing in quoted content, please let me know!
> Having descended onto a 30-part Mauro marathon, I can't a way to get
> back to the main screen showing all of the tracked patches...? "q" just
> drops me out entirely.
This is a gotcha. If you're in a review branch, we just show you the review
branch and skip the tracking app -- since we assume you just want to work on
the branch that you have checked out. There should be a big long-lasting
pop-up that tells you how to get back to the tracking app (switch back to a
non-review branch).
> When can we have it as an Emacs mode? :)
Uh...
> Thanks for all of this,
Thanks for trying it out!
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-03-16 23:28 ` b4 review available in master Jonathan Corbet
2026-03-16 23:41 ` Jonathan Corbet
2026-03-17 0:12 ` Konstantin Ryabitsev
@ 2026-03-18 13:43 ` Johannes Thumshirn
2 siblings, 0 replies; 69+ messages in thread
From: Johannes Thumshirn @ 2026-03-18 13:43 UTC (permalink / raw)
To: Jonathan Corbet, Konstantin Ryabitsev, users, tools
Am 17.03.26 um 00:28 schrieb Jonathan Corbet:
> When can we have it as an Emacs mode? 🙂
Not sure how useful it is to others but I've started a small b4-mode for
Emacs:
https://github.com/morbidrsa/b4-mode
b4 review isn't in there yet (only shazam and am which I use mostly).
Patches welcome.
Byte,
Johannes
^ permalink raw reply [flat|nested] 69+ messages in thread
* Re: b4 review available in master
2026-02-27 19:53 b4 review available in master Konstantin Ryabitsev
` (8 preceding siblings ...)
2026-03-16 23:28 ` b4 review available in master Jonathan Corbet
@ 2026-03-20 16:53 ` Louis Chauvet
2026-03-20 19:31 ` Konstantin Ryabitsev
9 siblings, 1 reply; 69+ messages in thread
From: Louis Chauvet @ 2026-03-20 16:53 UTC (permalink / raw)
To: Konstantin Ryabitsev, users, tools
On 2/27/26 20:53, Konstantin Ryabitsev wrote:
> Hi, all:
>
> TLDR: "b4 review" is a terminal UI that hopes to streamline a lot of
> maintainer duties. A getting-started guide with screencasts is here:
> https://b4.docs.kernel.org/en/latest/reviewer/getting-started.html
>
> It's available from b4 master pending initial testing and will be generally
> available in b4-0.15.
Hi everyone,
I’ve been testing the review tool and it’s working very well! Congrats
for this tool!
I’ve encountered a small issue while trying to review/test the following
series:
20260313132103.2529746-1-jim.cromie@gmail.com
When forcing drm-misc/drm-misc-next base, the check fails. I tried to
force the acceptation (^y), but I got a git am issue during the branch
creation (I fixed it manually, so my issue is "solved").
According to the documentation[1], it should use git am -3, so I tried
(in an other worktree, rerere disabled) to run b4 am -3 && git am -3,
and it worked (you need to fetch https://github.com/jimc/linux). I don't
understand why it doesn't work in b4 review.
Did I misunderstood something in the documentation? Do I have an issue
in my configuration? If not, can you add a way to apply the series with
git am -3?
Small list of "stuff I found when using the tool" (I totally accept a
"totally useless / don't have time / bad idea", the tool is already very
nice) :
- Add a way to see application logs (notably git errors)
- Add a way to read the existing trailers to mark patches as "done" (in
the above series, I already reviewed few patches, but they are not
marked as done in the tui).
- Copy don't work in the tui, it is a bit annoying (for example copy
git hash, text)
- When you exit, if a process was started in background (for example the
base check), they are not cancelled and the tool waits until completion
(not an issue for small series, but in my case the series is 63 patches,
so it can take up to 1 minute)
- Add a way to select and add trailers on multiple patches at the same time
- If a trailer is added in the `reply` interface, don't move it, I often
use and see stuff like `with or without this: R-by`, this is
"contextual", so could be nice to keep it in place.
- Add a config to disable "autosnip" (I understand the goal, but a per
patch "on/off" could be useful)
- Add a "quick shortcut" to add trailers (t -> r/a/t/n -> enter instead
of t -> arrows -> space -> enter)
- Add a way to disable the scroll animation (pgup/down do a "smooth"
scroll, which is very strange in a terminal) (very personal feeling)
- Scroll using arrows/pgupdown/jk don't work in the "send email"
confirmation list
Best regards,
Louis
[1]:https://b4.docs.kernel.org/en/latest/maintainer/review.html#conflict-resolution
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-03-20 16:53 ` Louis Chauvet
@ 2026-03-20 19:31 ` Konstantin Ryabitsev
2026-03-20 21:14 ` Alexandre Belloni
0 siblings, 1 reply; 69+ messages in thread
From: Konstantin Ryabitsev @ 2026-03-20 19:31 UTC (permalink / raw)
To: Louis Chauvet; +Cc: users, tools
Hi Louis,
Thanks for the thorough testing and the excellent feedback! I've committed a
number of fixes/changes in master that addresses a subset of them -- the
others will need to wait a bit, because I'm trying to get 0.15 out for wider
testing.
> When forcing drm-misc/drm-misc-next base, the check fails. I tried
> to force the acceptation (^y), but I got a git am issue during the
> branch creation
In theory, three-way merge (-3) is already enabled for all git-am operations
in the review TUI, so this should work in the current master. I don't know how
recent is the version you used for this, so hopefully the latest master is
already doing what you need. If you still see it fail, please send me the
exact error output and I'll take a look.
> - Add a way to see application logs (notably git errors)
For now, my recommendation is to press "s" to suspend the UI and drop to shell
-- you can review the b4 log output by scrolling up. Ctrl-D exits back into
the UI.
> - Copy don't work in the tui
This is a Textual framework limitation -- when mouse support is
enabled, the TUI captures mouse events for scrolling and clicking,
which prevents normal terminal text selection. Launch with
--no-mouse to get normal copy/paste, or make it permanent:
git config --global b4.review-no-mouse true
I've added a visible tip about this to the getting-started docs.
> - Add a "quick shortcut" to add trailers (t -> r/a/t/n -> enter
> instead of t -> arrows -> space -> enter)
Done! The trailer modal now accepts a/r/t/n as single-key toggles
for Acked-by, Reviewed-by, Tested-by, and NACKed-by.
> - Add a way to disable the scroll animation
The scroll animations come from Textual. You can disable them globally with an
environment variable:
export TEXTUAL_ANIMATIONS=none
or just smooth scrolling specifically:
export TEXTUAL_SMOOTH_SCROLL=0
I've also added a "Customising the TUI" section to the docs covering this and
other Textual tweaks (themes, colours, etc.).
> - Scroll using arrows/pgupdown/jk don't work in the "send email"
> confirmation list
Yeah, that was a bug. Fixed -- added j/k, arrows, space/backspace, and
pageup/pagedown bindings to the send confirmation modal.
> - Add a config to disable "autosnip"
This one is tricky -- there's a certain threshold where this more or less
becomes a "just open it and reply in your email client." :) I played a bit
with adding #!notrim and #!verbatim directives, but ended up with a bunch of
new bugs that I want to avoid in the roll-up to 0.15. So, I'm going to sit on
this for now and brainstorm about how and if we should implement such set of
features.
I've added the remaining items (auto-detect existing trailers, cancel
background processes on exit, batch trailer selection, keep trailers in-place
in reply editor) as planned for 0.16.
Thanks again!
Best,
--
KR
^ permalink raw reply [flat|nested] 69+ messages in thread* Re: b4 review available in master
2026-03-20 19:31 ` Konstantin Ryabitsev
@ 2026-03-20 21:14 ` Alexandre Belloni
0 siblings, 0 replies; 69+ messages in thread
From: Alexandre Belloni @ 2026-03-20 21:14 UTC (permalink / raw)
To: Konstantin Ryabitsev; +Cc: Louis Chauvet, users, tools
On 20/03/2026 15:31:06-0400, Konstantin Ryabitsev wrote:
> > - Copy don't work in the tui
>
> This is a Textual framework limitation -- when mouse support is
> enabled, the TUI captures mouse events for scrolling and clicking,
> which prevents normal terminal text selection. Launch with
> --no-mouse to get normal copy/paste, or make it permanent:
>
> git config --global b4.review-no-mouse true
>
> I've added a visible tip about this to the getting-started docs.
For this, I replied privately to Louis but shift+mouse should allow
copying, just like in neovim.
--
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 69+ messages in thread
