search

krobelus / git-branchstack

Efficiently manage Git branches without leaving your local branch
https://git.sr.ht/~krobelus/git-branchstack
MIT License
50 stars 2 forks source link

Preserve original commit timestamp when rebasing #5

Closed rf- closed 2 years ago

rf- commented 2 years ago

The goal here is to make the commit ID of the rebased commit consistent every time (both between topics and between git-branchstack runs), which combined with --trim-subject enables workflows like building chains of GitHub pull requests.

In a perfect world maybe we'd use the current user's name/email in the committer field and just keep the timestamp from the original commit, but it doesn't seem like there's a simple way to do that with git-revise right now. In practice I assume the author and committer will usually be the same person with this workflow anyway.

rf- commented 2 years ago

BTW I'm curious about the intended workflow without --trim-subject enabled, and specifically I was wondering whether it might make sense to make it the default. I guess maybe the idea is that reviewers can use the topic tags on the ancestor commits as a signal to ignore those commits when reviewing? It seems annoying to merge a branch that has those extraneous non-canonical commits in it though, right?

krobelus commented 2 years ago

I guess maybe the idea is that reviewers can use the topic tags on the ancestor commits as a signal to ignore those commits when reviewing?

Exactly.

It really depends on how people want to use it. We can definitely add a config option and maybe change default if there is a use case.

Though IME dependencies are not so common, so I wouldn't overthink it. I prefer submitting small, independent branches. If there are dependencies, then it's nice to clearly mark the fact (that this is not yet for merging). One thing that might help reviewers is to add links (to dependent PRs) instead of branch names. That could be scripted with git-revise -ie --ref some-branch.

Of course dependencies are really nice to quickly brew up experimental branches that combine my and others' changes.

So for my use I usually don't mind the topic tags - they are helpful reminders.

It seems annoying to merge a branch that has those extraneous non-canonical commits in it though, right?

Right. Sometimes two branches depend on the same commit (which for whatever reason is not merged separately). For that case I added the :+ dependency syntax as alternative to :, to imply --trim-subject.

krobelus commented 2 years ago

Thanks!

rf- commented 2 years ago

If there are dependencies, then it's nice to clearly mark the fact (that this is not yet for merging).

Ah, I see, this definitely helps me understand better. If you're assuming that you'll need to rebase and re-push a branch after merging its dependencies, it doesn't really matter whether it temporarily has tagged commits in it before then.

The cool thing about using --trim-subject and sharing commits between branches is that, when you do want a chain of related PRs, it plays better with GitHub's UI. You can make each PR use the previous one as its merge base, which has a couple of nice consequences:

I agree that it's not super common to need such a heavy workflow, but that's definitely the situation where I've most felt the need for a tool like branchstack, so I'm looking forward to taking advantage of it. Thanks for the merge!

krobelus commented 2 years ago

Right, in the common case where each topic has at most one dependency, the created branch can actually be based on its dependency (since we now reuse the committer date).

I totally forgot about targetting pull-requests against earlier branches in the chain. I used to do that but stopped because it confused people :( but it's the natural, obvious choice. Done with 8849303 (Make --trim-subject the default and add --keep-tags to not trim subjects, 2021-11-09)

krobelus commented 2 years ago

when using commit.gpgSign, we still get nondeterministic commit IDs by design. Then it's better to use git worktree add