Download raw body.
histedit/rebase vs author/committer times
On Wed, Jul 20, 2022 at 05:44:30PM +0200, Omar Polo wrote: > regarding the commit author vs committer turns out that we are already > doing a sensible thing (i.e. keeping the original author when rebasing > commits.) Sorry, I missed this patch and committed some overlapping changes, and proposed a conflicting diff for histedit/rebase. Let's peel apart which parts of our changes are not overlapping and continue based on that? > So, diff below just fixes the time accounting; the idea is: > > - use the current time when creating a new commit > - don't call time(NULL) twice > - when rebasing a commit use the old author_time and a new committer time > > from some tests this seems to put us in par with git, and I think that > it's a sensible behaviour. My changes cover don't add preservation of the 'author' timestamp, but cover the two other points you are addressing in pretty much the same way. I suspect what most people looking at timestamps will want to know is when a change arrived in the repository they are looking at. The author timestamp may well be entirely meaningless in that case. Since we always store the commit timestamp in the committer field, I suppose preserving the author's timestamp will not hurt. If we do this, we need to make sure that only the committer timestamp is used for comparison purposes, such as when sorting commits on parallel branches for linear display. And 'got log' and 'tog diff' only display one timestamp. This should be the committer timestamp in all cases. Or do you see a reason to display an author timestamp in any commands other than 'got cat'? > git (somehow) keeps the authorship also when "squashing" commits. (i.e. > you fold a commit from Alice with one did by you using 'git rebase -i' > and the resulting commit will still be from alice.) not sure if this is > my lazyness speaking, but i don't think `got histedit should follow the > same behaviour in this case. What likely happens here is that author field of the last commit in the folded series will be used? This is what would happen with my proposed rebase/histedit patch. > (going out of topic) it would be nice to have a mean to change the > commit metadata (author and committer) during histedit however. i was > thinking of adding `author' and `committer' that, like `mesg', would > allow to change those info in-line; i.e.: Yes, this could be useful in cases where the committer has made minor edits mixed into a patch submitted by someone else. It is best practice to commit a submitted patch as-is if possible, and then commit our own follow-up tweaks on top. But there could be cases where committing such a patch as-is could break things (e.g. break the build and thus make future bisecting harder), so this feature would indeed be useful. > pick abcdefg commit 1 > author Flan Hacker <flan@openbsd.org> > committer Omar Polo <op@omarpolo.com> Overriding the committer is redundant. We already set committer info based on values found in our run-time configuration. So I don't see how making it possible to override the committer field would be useful. Forcing people to re-type the entire author name and email is a bit heavy. I would only want to type it again if existing author info is incorrect or invalid (e.g. would fail the email address check, if such a commit was created somehow by some tool other than Got or Git). Should we also offer an alternative syntax which picks the author from a given (possibly abbreviated) commit ID, e.g. like this? fold abcdefg commit 1 fold 12345ae commit 2 pick 890abcd commit 3 author abcdefg
histedit/rebase vs author/committer times