jj
jj copied to clipboard
jj-vcs
docs: document code to avoid duplicating gerrit change-id
Adapted from:
- https://gist.github.com/thoughtpolice/8f2fd36ae17cd11b8e7bd93a70e31ad6#file-jjconfig-toml-L247
- https://github.com/avamsi/dotfiles/blob/28a0e40439de56fe179a1b0e705727c85e075464/.jjconfig.toml#L150
I'm not sure if I should add tests. I can add them if needed.
Checklist
If applicable:
- [ ] I have updated
CHANGELOG.md - [x] I have updated the documentation (README.md, docs/, demos/)
- [ ] I have updated the config schema (cli/src/config-schema.json)
- [ ] I have added tests to cover my changes
@mateusauler could you describe in what situation you already have a Change-Id trailer. Was it added by someone else?
Just trying to understand how often it might happen.
could you describe in what situation you already have a
Change-Idtrailer.
Which commands reprocess the description? (rebase, split, new x y, squash are some)
Does a signing of the commit affect it?
could you describe in what situation you already have a
Change-Idtrailer. Was it added by someone else? Just trying to understand how often it might happen.
This happens when the change's gerrit change-id does not match the one generated by jj. Eg. When a new change is pulled, jj assigns a new change-id. Which, in turn, makes jj generate a different gerrit change-id. Maybe we could generate jj's change-id based on gerrit's? I don't know if this is viable...
Which commands reprocess the description? (
rebase,split,new x y,squashare some)
describe, commit, squash, split, maybe more
Does a signing of the commit affect it?
If by signing you mean jj sign, then doesn't seem like it.
we can't easily know when searching a string in the description if it's actually a trailer.
Maybe we could add a .trailers() to the commit type?
We could implement the check in the rust code, with a configurable list of trailer to keep unique?
This could work, but how would that list be defined? In this case, a regex pattern or similar would be necessary, since the id is essentially random.
This happens when the change's gerrit change-id does not match the one generated by jj. Eg. When a new change is pulled, jj assigns a new change-id. Which, in turn, makes jj generate a different gerrit change-id. Maybe we could generate jj's change-id based on gerrit's? I don't know if this is viable...
I don't think that's possible: the id generated by format_gerrit_change_id_trailer in padded in order to match the gerrit id usual length. If we were to generate the jj change id from the gerrit change id, we would loose some data, and wouldn't be able to regenerate the gerrit change id. With the current trailer mechanism, the Change-id trailer would be added again, because it wouldn't exactly match the one already there.
we can't easily know when searching a string in the description if it's actually a trailer.
Maybe we could add a .trailers() to the commit type?
yes, that's an option @yuja suggested in the trailer PR
We could implement the check in the rust code, with a configurable list of trailer to keep unique?
This could work, but how would that list be defined? In this case, a regex pattern or similar would be necessary, since the id is essentially random.
We would only match the trailer key, and keep the value already there.
@yuja do you have a preference between the .trailers() method in the template or the config for the trailer key unicity? I think I prefer the first one, but I don't have a strong opinion. I could even give a try at implementing one or the other, once I'm done with jj split -A/-B/-d/-m — unless someone feels like implementing it already :-)
I don't think that's possible: the id generated by format_gerrit_change_id_trailer in padded in order to match the gerrit id usual length. If we were to generate the jj change id from the gerrit change id, we would loose some data, and wouldn't be able to regenerate the gerrit change id. With the current trailer mechanism, the Change-id trailer would be added again, because it wouldn't exactly match the one already there.
We could just ignore those first 8 characters in the gerrit change-id. Both when checking for the trailer and generating jj's change-id after a pull. Since jj's template always generates the same ones, from our perspective, they're irrelevant even if they're not 6a6a6964.
We would only match the trailer key, and keep the value already there.
Makes sense.
unless someone feels like implementing it already :-)
I might try hacking on it a bit over the weekend. No promises though...
We would only match the trailer key, and keep the value already there.
@yuja do you have a preference between the
.trailers()method in the template or the config for the trailer key unicity? I think I prefer the first one, but I don't have a strong opinion.
I think config is better if that's sufficient and common requirement. If it's rarely needed, if(!trailers.contains("Change-Id"), format_gerrit_change_id_trailer(..)) seems good.
.trailers() will be useful as its own (to render trailers differently in jj show for example), but it's inconsistent that we deduplicate new trailers by (key, value) match, whereas the user has to do that manually for key match.
