Squash commits

[peacechen]

@oze4 I'm debugging a regression that was introduced since the 3.x release (within the last few months). This has become increasingly difficult with the branch merge strategy. I highly recommend squash commits for all branch merges because it simplifies narrowing down bugs. Retaining 10+ commits from a branch is of little value when they contain experimentation and tiny fixes in those numerous commits.

[matthewoestreich]

Thank you for the suggestion.

[peacechen]

The bug I chased down for the columns copy took 2-3x longer than it should have. Instead of referencing a commit on the main branch, it was hidden in a branch from a month prior to the merge commit. From my experience on large projects, commits on the main branch should be atomic per-feature or bug fix instead of spread out to another branch at another time.

[matthewoestreich]

Yea that's my fault (prob) as I know I was committing directly to the main branch vs creating a branch and then merging. I'm pretty sure I was the only one that did that so I take full responsibility. Domino brought that up to me as well a couple months ago and I've been trying to shake that bad habit. Sorry all :( Rookie move.

[matthewoestreich]

Oh wait are you saying we shouldn't be creating branches for features/fixes/etc...? And we should be committing directly to the main branch?

Which is "best practices"?

1. To create a branch, apply changes, then merge
2. Commit directly to the main branch

[peacechen]

There are 2 primary scenarios.

1. External contributors via PR
2. Direct contributors (you & domino)

For external contribs, choose the Fast-Forward Squash commit instead of merge commit. This can be set as default in the project settings on Github.

For internal contributors, it's often better to create a feature branch. When you're done with it, manually squash the commits with git (command line or your favorite git client). Then do a fast-forward merge to pull the squashed commit to the head of main branch. You can also do a regular merge from the branch which leaves an extra anonymous branch out there. I recommend rebasing all branches before merging so that the merge commit doesn't end up with all the intermediary changes.

I don't believe you can create a PR from a branch on the same repo. If you can, choose the same Squash commit option as for external contribs.

[matthewoestreich]

Squashing the commits plus rebasing makes sense. You can merge a PR for the same repo with a diff branch. That's how I've been doing it. But yeah I see what you mean and how that could have been painful tracking down.

[peacechen]

Here's a branch that's well-contained time-wise, but has a lot of intermingled branch merges. Those commits aren't historically useful. It's more effective to have a single commit that represents the entire feature.

In the case of @FalkF's branch, he branched it on 6/19/2021, made some commits, and in between merged from main not once, not twice, but five times before it was merged into main on 8/13/2/201. The intermediary commits and merges over the months are difficult to follow and again, not historically useful. A squash commit eliminates all of that noise.

[matthewoestreich]

What permissions do you need to this repo in order to make those changes? If @Domino987 has no objections I'm open to letting you make these changes.

If you do make those changes, will it cause issues with currently published versions?

Is it pointless to retroactively make these changes (is this something we should just do moving forward or do we need to remedy things retroactively)?

[Domino987]

Yes this looks like a good way to reduce the commit noise. I am sure this would make the tracking easier. I also think since you are an active member since a long time now, it is overdue to give you merge permissions as well, if @oze4 agrees. So we can support this lib even more.

[peacechen]

I'd be happy to help out as time permits. I can set it if you'd like to give me permissions. It's under the Settings tab at the top of the main page, Options tab on the left side, then Merge Button towards the bottom of the page.

Changing those settings doesn't affect past commits or merges. It will apply to future merges.

Retroactively applying those changes would rewrite history. It can be done, but would be very involved and require advanced git-fu. I've done it before, but usually not worth it.

[matthewoestreich]

I'll get this done as soon as I'm back by my laptop. Thank you so much!

[peacechen]

I'll also add that changing the merge strategy from merge to FF rebase requires fixing merge conflicts during the rebase. This doesn't add or remove that step, since merging also requires resolving conflicts. It merely moves the point at which conflicts need to be resolved.

Edit: Squash commits should be preferred over rebase. Rebase retains all of the commits on the branch, which doesn't help reduce the noise.

[matthewoestreich]

@peacechen

I went ahead and added you as a maintainer to this repo. You should be able to make the necessary git changes now (as well as merge PRs that I believe you submitted - if you want to merge you can).

cc @Domino987

[peacechen]

Thanks @oze4
I changed the setting to Squash commits only, disabling merge and rebase which lead to the snarl of branches retained in history.

[peacechen]

@oze4 I've been thinking about your question

Is it pointless to retroactively make these changes

Doing so involves rewriting history and is a time-consuming manual task. That said, the noise from the merge commits 8/12 - 8/13 is significant and would be easy to clean up. If you're open to it, I can do the following locally:

1. Reset main branch to 7ef4880 committed on 8/7 before the batch of merge commits.
2. Cherry pick each merge commit, effectively squashing them after the fact.
3. Continue to the head of main to retain all commits.

When that is done, I would force push the main branch to rewrite history. After this occurs, any of your outstanding branches should be rebased to head of main. If you have local work not on a branch, stash them, FF pull, then apply the stash. Don't do a merge pull or you'll re-introduce the snarl of branches.

Any branches based on commits prior to 8/7 won't be affected.

Give me the word if you'd like me to do that. It should take less than 15 minutes.

[matthewoestreich]

I think you know how this should be handled and I do not so you have my support.

With that said @Domino987 needs to buy off on any and all changes as well. I'm just more outspoken than him which is prob why you see my name a lot :) but we are partners in this project together. This thing wouldn't be half as stable without him (just trying to convey how important he is to this project lol).

So @Domino987 - thoughts?

[matthewoestreich]

It goes without saying : thank you very much for your interest in helping to improve this awesome project @peacechen

[peacechen]

You're very welcome. I'm happy to help. I've gotten more out of this project than I've put in.

@Domino987 are you good with me squashing branches from the last ~4 merges?