| Message ID | 20240524-doc-send-prefixes-v1-1-c6465822ad8c@cherry.de (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
| Series | [b4] docs: document b4.send-prefixes conf option | expand |
On Fri, May 24, 2024 at 03:30:35PM GMT, Quentin Schulz wrote: > I'm also wondering if we're missing tests for this? We're missing tests for most things. :) Writing tests is a lot of work and, unfortunately, I do not often have enough time to spend on it. There are some minimal tests in place for the features that, if broken, will result in maintainers being very upset. > Additionally, should it really be entirely replaced with > `b4 prep --set-prefixes` rather than appended? I'm happy to discuss this. A lot of features start out as stubs that later develop nuance as people start using them. > I'm trying to add config files for b4 to different Yocto git repos. > Small Yocto git repos do share the same mailing list, but need to use a > prefix. However, in Yocto we have a branch per release, so one would > need to explicitly add the branch(es) to which this patch series apply > to as additional prefixes. This would mean we would have to add the repo > prefix again through `b4 prep --set-prefixes`. This is, indeed, different from the kernel -- which is why you are having trouble adapting the feature as it exists to your use-case. I'm happy to brainstorm about this to figure out what would make it most convenient for your needs. One way I can think of to address your problem is to allow a + to indicate that a prefix should be added to any existing ones (if not already present). This would allow one to run: b4 prep --set-prefixes +foo and also specify in .b4-config: [b4] send-prefixes = +foo It's kinda hackish, but effective. > Also, modifying .b4-config with an already-created series will not > update the prefixes (which made me first believe this was not supported > at all). Not sure how to handle this though (if we even want to)? We can specifically treat any +foo prefixes as "always add if not present at send time". -K
On Fri, May 24, 2024 at 10:28:59AM GMT, Konstantin Ryabitsev wrote: > One way I can think of to address your problem is to allow a + to > indicate that a prefix should be added to any existing ones (if not > already present). This would allow one to run: > > b4 prep --set-prefixes +foo > > and also specify in .b4-config: > > [b4] > send-prefixes = +foo > > It's kinda hackish, but effective. I take it back -- thinking about it with some more coffee in, this would break things badly for people with old versions of b4. I think a better plan is to: 1. introduce an --add-prefixes switch that specifically adds to prefixes 2. if there are prefixes defined in b4.send-prefixes, we *always* insert them if not already present 3. switch prefix management to always be in the subject line of the cover letter, as opposed to being stored in the tracking json -K
Hi Konstantin, On 5/24/24 4:49 PM, Konstantin Ryabitsev wrote: > On Fri, May 24, 2024 at 10:28:59AM GMT, Konstantin Ryabitsev wrote: >> One way I can think of to address your problem is to allow a + to >> indicate that a prefix should be added to any existing ones (if not >> already present). This would allow one to run: >> >> b4 prep --set-prefixes +foo >> >> and also specify in .b4-config: >> >> [b4] >> send-prefixes = +foo >> >> It's kinda hackish, but effective. > > I take it back -- thinking about it with some more coffee in, this would > break things badly for people with old versions of b4. I think a better plan > is to: > Exactly, one would have '+' added with old versions of b4, not great. > 1. introduce an --add-prefixes switch that specifically adds to prefixes Yup, that was something that went through my mind too. If we want to add a b4 prep --add-prefixes or edit a current b4 series add-prefixes, we need a way to be able to remove those I assume... One way is to manually edit the `b4-submit-tracking` part by hand with git rebase but I think there's a reason b4 prep --edit-cover doesn't give you this part of it :) --remove-add-prefixes X Y Z? --reset-add-prefixes? Or maybe we care about this once someone complains about it? > 2. if there are prefixes defined in b4.send-prefixes, we *always* insert them > if not already present Considering the option in b4 prep has a different name than the one in b4-config, that could be not TOO confusing, so that's an option. Though... inconsistent behavior between versions of b4 again :/ So need to document different workflows for different versions of b4, not sure it's a good idea. > 3. switch prefix management to always be in the subject line of the cover > letter, as opposed to being stored in the tracking json > That's an option but then it'd appear in git log. Additionally, I'm not entirely sure everyone wants to blindly have all strings matching '[.*]' in the subject line be applied to all other patches in the series? I would go for 1., probably because I thought of it as well and that 3. may add a lot of corner cases and I'm not sure how to handle them? BTW, I also had a "surprising" issu when the cover letter title is the same as a patch and I added a git fixup, a git rebase --autosquash would try to fixup the cover letter rather than the commit, which makes sense, but was not expecting a git conflict at that time :) So if we go for 3. maybe that's also a way to add some logic there for that, if that is a "problem" worth fixing. Cheers, Quentin
On Fri, May 24, 2024 at 05:07:53PM GMT, Quentin Schulz wrote: > > 3. switch prefix management to always be in the subject line of the cover > > letter, as opposed to being stored in the tracking json > > > > That's an option but then it'd appear in git log. Not sure how you mean. Yes, it's in the git log, but since it's just the cover commit -- it doesn't really matter? > Additionally, I'm not > entirely sure everyone wants to blindly have all strings matching '[.*]' in > the subject line be applied to all other patches in the series? Well, it's stricter than that -- it has to specifically be at the beginning of the subject. And wouldn't you always want the same prefixes across the entire series? > I would go for 1., probably because I thought of it as well and that 3. may > add a lot of corner cases and I'm not sure how to handle them? There's another option of just using --edit-prefixes the same way we do --edit-deps, but it seems a bit overkill to kick off an editor just to edit a couple of words. :) > BTW, I also had a "surprising" issu when the cover letter title is the same > as a patch and I added a git fixup, a git rebase --autosquash would try to > fixup the cover letter rather than the commit, which makes sense, but was > not expecting a git conflict at that time :) So if we go for 3. maybe that's > also a way to add some logic there for that, if that is a "problem" worth > fixing. So, you mean the cover letter should always have a subject like: [COVER] This is the cover letter subject That could both help visually distinguish it in a git interactive rebase session, and then clearly show where prefixes should be added. -K
Hi Konstantin, On 5/24/24 10:25 PM, Konstantin Ryabitsev wrote: > On Fri, May 24, 2024 at 05:07:53PM GMT, Quentin Schulz wrote: >>> 3. switch prefix management to always be in the subject line of the cover >>> letter, as opposed to being stored in the tracking json >>> >> >> That's an option but then it'd appear in git log. > > Not sure how you mean. Yes, it's in the git log, but since it's just the cover > commit -- it doesn't really matter? > Not sure anymore either, too late on Friday I guess :/ >> Additionally, I'm not >> entirely sure everyone wants to blindly have all strings matching '[.*]' in >> the subject line be applied to all other patches in the series? > > Well, it's stricter than that -- it has to specifically be at the beginning of > the subject. And wouldn't you always want the same prefixes across the entire > series? > Yeah, I guess this would be fine actually. >> I would go for 1., probably because I thought of it as well and that 3. may >> add a lot of corner cases and I'm not sure how to handle them? > > There's another option of just using --edit-prefixes the same way we do > --edit-deps, but it seems a bit overkill to kick off an editor just to edit a > couple of words. :) > >> BTW, I also had a "surprising" issu when the cover letter title is the same >> as a patch and I added a git fixup, a git rebase --autosquash would try to >> fixup the cover letter rather than the commit, which makes sense, but was >> not expecting a git conflict at that time :) So if we go for 3. maybe that's >> also a way to add some logic there for that, if that is a "problem" worth >> fixing. > > So, you mean the cover letter should always have a subject like: > > [COVER] This is the cover letter subject > That sounds right to me? > That could both help visually distinguish it in a git interactive rebase > session, and then clearly show where prefixes should be added. > One can identify the cover letter in a rebase, because it has a "#empty" at the end of the line in interactive mode. It's just that the --autosquash doesn't handle this properly and takes (I assume) the first commit in reverse history (older first) title that matches exactly what's after !fixup in the fixup commit, trying to fixup the cover letter instead of the actual interesting commit. Have we agreed on solution 3) then? What exactly is next? Just to know what are the expectations from you and me? Cheers, Quentin
On Mon, May 27, 2024 at 10:20:39AM GMT, Quentin Schulz wrote: > One can identify the cover letter in a rebase, because it has a "#empty" at > the end of the line in interactive mode. It's just that the --autosquash > doesn't handle this properly and takes (I assume) the first commit in > reverse history (older first) title that matches exactly what's after !fixup > in the fixup commit, trying to fixup the cover letter instead of the actual > interesting commit. > > Have we agreed on solution 3) then? What exactly is next? Just to know what > are the expectations from you and me? For the moment, my plan is: 1. change b4.send-prefixes to be always additive 2. introduce the --add-prefixes command 3. leave the rest as-is right now I should be able to get this into 0.14. -K
Hi Konstantin, On 5/24/24 10:25 PM, Konstantin Ryabitsev wrote: > On Fri, May 24, 2024 at 05:07:53PM GMT, Quentin Schulz wrote: >>> 3. switch prefix management to always be in the subject line of the cover >>> letter, as opposed to being stored in the tracking json >>> >> >> That's an option but then it'd appear in git log. > > Not sure how you mean. Yes, it's in the git log, but since it's just the cover > commit -- it doesn't really matter? > >> Additionally, I'm not >> entirely sure everyone wants to blindly have all strings matching '[.*]' in >> the subject line be applied to all other patches in the series? > > Well, it's stricter than that -- it has to specifically be at the beginning of > the subject. And wouldn't you always want the same prefixes across the entire > series? > >> I would go for 1., probably because I thought of it as well and that 3. may >> add a lot of corner cases and I'm not sure how to handle them? > > There's another option of just using --edit-prefixes the same way we do > --edit-deps, but it seems a bit overkill to kick off an editor just to edit a > couple of words. :) > >> BTW, I also had a "surprising" issu when the cover letter title is the same >> as a patch and I added a git fixup, a git rebase --autosquash would try to >> fixup the cover letter rather than the commit, which makes sense, but was >> not expecting a git conflict at that time :) So if we go for 3. maybe that's >> also a way to add some logic there for that, if that is a "problem" worth >> fixing. > > So, you mean the cover letter should always have a subject like: > > [COVER] This is the cover letter subject > Just FYI, I tested this locally with b4 v0.13 [COVER] in the cover letter makes all commits in the patch series have COVER as a prefix. So it seems that some piping is already in place (on purpose or through some side effect). Though we may want to not propagate [COVER] prefix to all commits in the series when sending with b4 send. Cheers, Quentin
On Fri, 24 May 2024 15:30:35 +0200, Quentin Schulz wrote: > b4.send-prefixes is a way to define a set of default prefixes from a b4 > configuration file, so let's document it. > > This is available since commit c42dbb0e5040 ("ez: reimplement extra > prefix functionality"), part of v0.11 tag. > > > [...] Applied, thanks! [1/1] docs: document b4.send-prefixes conf option commit: b96a6e5e55ab8a6b458915f0255da956d1de8126 Best regards,
diff --git a/docs/config.rst b/docs/config.rst index cd3d151..56a1fe4 100644 --- a/docs/config.rst +++ b/docs/config.rst @@ -414,6 +414,13 @@ Contributor-oriented settings Default: ``None`` +``b4.send-prefixes`` (v0.11+) + Extra prefixes to add to ``[PATCH]`` (e.g. ``RFC mydrv``). + + This setting can be replaced for a series with ``b4 prep --set-prefixes``. + + Default: ``None`` + ``b4.prep-perpatch-check-cmd`` (v0.14+) The command to use when running ``--check``. If b4 finds ``scripts/checkpatch.pl`` at the top of your git tree, it uses the