As a developer, I need documentation on "to rebase or not" in feature branches.

[poikilotherm]

Dear devs, QA and anybody else who it may concern,

I would be very glad if some documentation about how to handle a diverged develop branch in my feature branch when creating or updating a PR before merge.

Seeing a lot of "merge" commits in the codebase and personally not being a fan of those if they are caused by devs merging in develop instead of rebasing and cleaning up conflicts in their commits, I would really appreciate some guidance on this.

Enforcing a standard or rule on this is load on QA, but IMHO it is worth it. Open for discussion :-) Again, I would really appreciate feedback, as I am currently rebasing, which has its advantages and disadvantages (just like merging has).

[pdurbin]

@poikilotherm thanks for opening this issue. For now I'll just encourage you to read through #3418, especially the part where I mention my desire for a "clean git history" but you have to understand that we're still relatively new to git and many of us used SVN for years and years. Only recently have we started using branches and pull requests per feature. We are slowly evolving and maturing and feedback like this is healthy for us. Thanks.

[poikilotherm]

Looks like this is nothing too many people care about. 🤷 Closing for cleanup.

[pdurbin]

I just thought I'd mention that @adaybujeda for example likes to rebase his pull requests. This is different that what we do internally (we always just merge develop into our branches). So we're starting to gain experience with both approaches. I like the idea of a clean git history but I'm fine with merging or rebasing. It's all good. 😄

[pdurbin]

From @adaybujeda at #7325 (comment)

"For PRs, I tend to squash all my changes into a single commit. Usually, when new changes/updates need to be made to the pr, I will rebase the branch, reset my commit and make a new one with the latest changes/updates. I find it easier to review and to reason about the changes this way."

The context is that we're thinking about removing a change from a PR and we don't want that removal to apply to another PR down the road.

Overall, I like the idea that squashing and rebasing would provide a clean git history. There's an image of this here: https://blog.carbonfive.com/always-squash-and-rebase-your-git-commits/

One concern I have with squashing and rebasing is that it's harder for the reviewer to see what changed after the reviewer gives feedback and the developer changes something. There's no diff to look at. And the reviewer probably doesn't remember exactly how the old code looked.

As we say in the dev guide, we're following https://nvie.com/posts/a-successful-git-branching-model/ and it doesn't mention rebasing at all. That's not to say that we can't start getting into squashing and rebasing, of course. However, we should probably write up some guidance in the dev guide if we want to encourage it. Here's our guidance as of this writing: https://guides.dataverse.org/en/5.11.1/developers/version-control.html

I guess I'll re-open this issue for now. 😄

[qqmyers]

I'd vote no on squashing in general - being able to use blame to find the source of a line of code and to see the small set of changes made when it was added has been extremely valuable in finding/fixing bugs, including for the recent 5.11.1 hotfix. While looking at the overall PR is also helpful, that's often too big a chunk of work to analyze. FWIW: I sometimes use squash when I make a change and fairly quickly find a minor bug/issue and think a quick squash will help in future understanding of that change.

[poikilotherm]

As you might have noticed, I am using https://www.conventionalcommits.org for quite a while now. I'm trying to shape useful units of changes and add a comment about the "why" so git blame can play out its strength. I'm also using partial commits for this. (git add -p or nice UI via IntelliJ)

For this reason, I completely agree with @qqmyers - I'm not a fan of completely squashed histories.

As Jim also mentioned, when I discover a change should be part of an earlier commit (forgot to include a particular line, etc), I tend to fix up the commit. If it's the last one that's easy with git commit --amend. For commits in the past I use the "git fixup workflow" a lot, including interactive rebase squashing for history rewrite. (So some squashing, but not squashing all commits of a PR into a single commit.)

FWIW, this is my git fixup alias: !f() { TARGET=$(git rev-parse $1); git commit --fixup=$TARGET ${@:2} && GIT_SEQUENCE_EDITOR=true git rebase -i --autostash --autosquash $TARGET^; }; f

[cmbz]

To focus on the most important features and bugs, we are closing issues created before 2020 (version 5.0) that are not new feature requests with the label 'Type: Feature'.

If you created this issue and you feel the team should revisit this decision, please reopen the issue and leave a comment.