b4: use new trailers in 'b4 send --resend'
Hello,
first of all: thanks for b4, I love it when sending my patches!
I found a little issue when resending a patch series with unmodified content, but with new trailers received after the initial sending.
This are the events involved:
1. send a patch series (b4 send)
2. receive some Reviewed-by, but patch not applied and no changes needed
3. update trailers (b4 trailers -u)
4. resend (b4 send --resend)
At step 4 I would expect b4 to prepare a [PATCH RESEND] series with the new trailers added. Instead b4 resends the exact same series, without any new trailers.
According to my experience in contributing to the Linux kernel, having the trailers added when resending is the primary goal when resending a series that needs no other changes, so I think it should be done by b4 automatically.
Best regards,
Luca
Signature · 3 lines
Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Quoted 13 lines · Luca Ceresoli
first of all: thanks for b4, I love it when sending my patches!
I found a little issue when resending a patch series with unmodified content, but with new trailers received after the initial sending.
This are the events involved:
1. send a patch series (b4 send)
2. receive some Reviewed-by, but patch not applied and no changes needed
3. update trailers (b4 trailers -u)
4. resend (b4 send --resend)
Quoted 3 lines
At step 4 I would expect b4 to prepare a [PATCH RESEND] series with the new trailers added. Instead b4 resends the exact same series, without any new trailers.
This is the expected behaviour, as far as I know. There should be no differences between the series, otherwise it's not really a resend.
It's not really necessary to resend a series just for trailer changes anyway. The maintainer will most likely use b4 to retrieve it anyway, and that will pull in any new trailers.
-K
Hello Konstantin,
thanks for your quick feedback.
Quoted 18 lines · On Fri, 26 Apr 2024 11:11:29 -0400 Konstantin Ryabitsev…
Quoted 13 lines · Luca Ceresoli
first of all: thanks for b4, I love it when sending my patches!
I found a little issue when resending a patch series with unmodified content, but with new trailers received after the initial sending.
This are the events involved:
1. send a patch series (b4 send)
2. receive some Reviewed-by, but patch not applied and no changes needed
3. update trailers (b4 trailers -u)
4. resend (b4 send --resend)Quoted 3 lines
At step 4 I would expect b4 to prepare a [PATCH RESEND] series with the new trailers added. Instead b4 resends the exact same series, without any new trailers.
Quoted 6 lines
This is the expected behaviour, as far as I know. There should be no differences between the series, otherwise it's not really a resend.
It's not really necessary to resend a series just for trailer changes anyway. The maintainer will most likely use b4 to retrieve it anyway, and that will pull in any new trailers.
I cannot recall exactly when it did happen and provide a link right now, but I clearly remember having been asked by some maintainer to do exactly this, instead of pinging.
The rationale is that sending a new version just to add review tags was considered bad because one can expect the patches or commit description to have changed somewhat, which it hasn't.
OTOH resending without new trailers can lead to applying a patch without those tags (not recognizing reviewers time), or even worse to discard a patch because it does not show the needed tags.
It all made sense to me. Does it look reasonable to you too, with this extra explanation?
Best regards,
Luca
Signature · 3 lines
Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Quoted 11 lines · Luca Ceresoli
Quoted 3 lines
It's not really necessary to resend a series just for trailer changes anyway. The maintainer will most likely use b4 to retrieve it anyway, and that will pull in any new trailers.
I cannot recall exactly when it did happen and provide a link right now, but I clearly remember having been asked by some maintainer to do exactly this, instead of pinging.
The rationale is that sending a new version just to add review tags was considered bad because one can expect the patches or commit description to have changed somewhat, which it hasn't.
Yes, to be clear -- I wasn't suggesting that you send a new revision just to add trailers either. We're both aligned here.
Quoted 3 lines
OTOH resending without new trailers can lead to applying a patch without those tags (not recognizing reviewers time), or even worse to discard a patch because it does not show the needed tags.
I expect this is one of those "every subsystem does things slightly differently" situation. If the maintainer uses b4, this entire step is unnecessary, because any missing trailers will be applied anyway when they run "b4 am". If they don't use b4 -- or if they use a very old version of b4 which didn't match by patch-id to collect trailers sent to other identical submissions -- then they would have a reason to complain.
I may have to run a poll among maintainers to see which is the behaviour they want to see:
- resend should be identical to what was sent the first time
- resend should include any additional trailers received since the initial send-out
-K
Hello Konstantin,
Quoted 16 lines · On Fri, 26 Apr 2024 14:04:49 -0400 Konstantin Ryabitsev…
Quoted 11 lines · Luca Ceresoli
Quoted 3 lines
It's not really necessary to resend a series just for trailer changes anyway. The maintainer will most likely use b4 to retrieve it anyway, and that will pull in any new trailers.
I cannot recall exactly when it did happen and provide a link right now, but I clearly remember having been asked by some maintainer to do exactly this, instead of pinging.
The rationale is that sending a new version just to add review tags was considered bad because one can expect the patches or commit description to have changed somewhat, which it hasn't.
Yes, to be clear -- I wasn't suggesting that you send a new revision just to add trailers either. We're both aligned here.
Quoted 8 lines
Quoted 3 lines
OTOH resending without new trailers can lead to applying a patch without those tags (not recognizing reviewers time), or even worse to discard a patch because it does not show the needed tags.
I expect this is one of those "every subsystem does things slightly differently" situation. If the maintainer uses b4, this entire step is unnecessary, because any missing trailers will be applied anyway when they run "b4 am".
Ah, interesting feature. I had no idea about it as I'm using b4 for sending patches only. Thanks for the clarification!
Luca
Signature · 3 lines
Luca Ceresoli, Bootlin Embedded Linux and Kernel engineering https://bootlin.com
Quoted 6 lines · Konstantin Ryabitsev
Quoted 3 lines · Luca Ceresoli
Quoted 3 lines
It's not really necessary to resend a series just for trailer changes anyway. The maintainer will most likely use b4 to retrieve it anyway, and that will pull in any new trailers.
Quoted 6 lines
Quoted 3 lines
I cannot recall exactly when it did happen and provide a link right now, but I clearly remember having been asked by some maintainer to do exactly this, instead of pinging.
Yes, to be clear -- I wasn't suggesting that you send a new revision just to add trailers either. We're both aligned here.
This sounds like something I routinely say, though not in order to collect trailers but rather because if something went AWOL the basic answer to a ping is going to be a "I have no idea what you're talking about" so it's just easier to send the thing again combined with a general desire to discourage people from sending content free pings.
Quoted 6 lines
I may have to run a poll among maintainers to see which is the behaviour they want to see:
- resend should be identical to what was sent the first time
- resend should include any additional trailers received since the initial send-out
One possible issue with not having the trailers in a new thread would be that you might not get as far as fetching the patches with b4 if it looked like they'd not got enough review. There are also people who get really annoyed when they see something that looks like a submitter is dropping their acks or reviews needlessly, among other things it does mean people can end up repeating review they've already done.