I think that version number is supposed to be bumped when you change format of the file in a way that breaks existing readers (i.e. by changing to a non-cbor format?). The current user (hg graft) always passes version=1 and doesn't care what the version is when it reads the file.
I mostly have various questions, some being related to the question in the previous changeset:
- why do we even need a temporary commit instead simply using update logic directly
- Can you clarify the business about graft, version etc (see my comment on`_continue_amend_rev`)
- If we need to rebase a changesets, why do we need to directly call the rebase extensions instead of using some common factored out code ? (or at least maybe hg graft ?)
- Can we get more docstring about the role and semantic of the various new functions ?
Can't we use the rewriteutil do to this instead of relying on a third party extension ?
Are we not using rebase here instead of graft ?
And in all case: why does the rebase/graft behavior matters here ? If we are falling back to the logic of another extension/command¹ why isn't that command dealing with its state file itself instead of use having to mimic it?
Or maybe I am missing something here.
 which as find hacky
What's the difference between continue_amend_rev and do_continue_amend_rev which part of the continuation is it actually doing ? I think having some docstring for these functions would help.
Same feedback about doc-string applies.
Should we filter out None value explicitly here instead of relying on the next loop to get rid of it ?
small nits: I would advocate for intermediate variable to be used instead of a statement spawning over four line.
same minor nits here.
Maybe. I've never seen rewriteutil used for rebasing commits. Can you explain?
(rebase has been a first-party extension for a long time, as far as I know)
This is about the state saved by the amend extension, not state saved by either rebase or graft. The only reason I mentioned hg graft was to answer Alphare's question about version.
Good point. Hopefully the added docstrings help. Let me know otherwise, of course.
I agree that it would seem like it and that's what I did initially, but it becomes annoying to do that after the next patch.
The current style matches the surrounding functions. I'll send a separate patch (either before or after this one) to extract the message string to separate variables
There seems to have been no activities on this Diff for the past 3 Months.
By policy, we are automatically moving it out of the need-review state.
Please, move it back to need-review without hesitation if this diff should still be discussed.