additional merge options for git ship

[FFdhorkin]

Some context from other issues

Git Town's git ship command currently always squash-merges branches because that's the right thing for the vast majority of users. One of Git Town's paradigms is to make Git easier to use correctly. Another paradigm is "when in doubt keep it out" to keep Git Town simple and easy to learn. When somebody comes up with a convincing use case for another merge method for shipping, we will add it. Just saying "GitHub also supports it" is a valid data point but by itself not convincing enough in this context.

Originally posted by @kevgo in #2324 (comment)

Ideally even commits that address changes get squashed away here. If we don't do this aggressively, we run into the issue that somebody clicks the big green "merge" button on GitHub, and we mess up the Git history with WIP commits.

Originally posted by @kevgo in #673 (comment)

Request

There should be a configuration option for git ship that determines strategy (i.e. choose between rebase, merge commit, and squash).

A few related ideas for further enhancement:

• a config option that scans commits-to-be-shipped for "WIP" in the title (or perhaps titles with < X characters), and if this check fails either:
  • give an error & refuse to do anything
  • give a prompt asking if you would like to squash; if the answer is no, then refuse to do anything
• a config option that asks you whether you'd like to squash if the size is < X lines of code, but would defer to your git ship strategy if the line limit is exceeded

These changes would ensure quality commit history without over-reliance on squash merges, which, IMHO, are bad the majority of the time and indicative of antipatterns.

Background

It seems that Git Town design assumes that commit history will be cluttered with WIP commits and that squash commits are the right way to go. However, I consider pushing WIP commits to be an anti-pattern - I would reject any PR that contains any WIP commits unless it's a good candidate for squashing (atomic and < ~150 lines changed. Atomic as in "no unrelated changes in the same commit").

If I saw a PR with a bunch of WIP commits (or a single massive commit containing a bunch of unrelated changes), I would tell the submitter to revise their git history and resubmit. Commit history should be useful - that means you have meaningful commit messages and that your commits have some sort of logical grouping. A good commit history should let you review a PR one commit at a time in the event that a branch has a significant number of changes (incidentally, this is basically the gerrit code review workflow, and it is something I VERY much miss using)

Squash commits are a bad idea most of the time, because they intermingle unrelated changes and make it harder to review code. If you have a commit about updating documentation and another commit about fixing totally unrelated bugs, but they are otherwise grouped in a single PR because there's some logical reason for it, then squashing means that you have a bugfix combined with your doc updates and can no longer consider them individually. Worse, if your PR has a large number of lines changed, a squash merge means you might have a PR with 1000+ lines changed and no way to make it more atomic by looking at commits individually.

An additional problem with squash commits is that if you need to revert a change, you can't revert just one small commit, you have to revert the entire squashed history and then manually pick through and discard changes you don't want to revert.

TL;DR, I would reject any PR that contained WIP commits over ~150 lines and/or had a single commit with a ton of unrelated changes. By and large, I agree with the guidelines that are described at Git Commit Best Practices

What is my typical workflow?

The way I tend to work looks something like this:

• make incremental WIP commits locally so that i have some archival history in case I break something in a later commit, etc
• once I've reached some kind of completed logical unit, I either squash my related WIP commits, or do an interactive rebase to reorder various WIPs and then squash them, so that things are tidy

As a more concrete example, this is roughly how I tend to open new repos:

• make some wip project setup commits for setting up the basic project stuff (package.json, tsconfig.json, linting, hooks, etc)
• start adding the "initial" level functionality (commit messages like "wip feature foo")
• as I flesh out functionality, I realize I needed to change things in my config files, add npm scripts, etc, so I add commits with messages like wip update config
• when I finally reach the point where I'm ready to consider a code review, I do an interactive rebase that reorders and squashes and updates the message

This approach also minimizes churn - most of my final commits tend to be additive, as opposed to the same line of code changing multiple times. So it is generally possible to review one commit at a time more quickly than the flattened PR, since related changes are grouped.

Click me for a concrete mock example of the above

I start with a commit history like

• wip init package
• wip setup typescript
• wip add linting
• wip add commit hook
• wip feature foo
• wip tweak typescript config
• wip feature foo
• wip update package.json
• wip feature foo
• wip tweak eslint rules
• wip update commit hook
• add test script to package.json

then rearrange to

• wip init package
• wip update package.json
• add test script to package.json
• wip setup typescript
• wip tweak typescript config
• wip add linting
• wip tweak eslint rules
• wip add commit hook
• wip update commit hook
• wip feature foo
• wip feature foo
• wip feature foo

and finally squash related commits:

• Initialize package.json and base dependencies
• Configure Typescript project
• Add eslint linting config
• Add precommit hook
• Add foo feature

[kevgo]

Thanks for bringing this up, I agree that the ship behavior should be configurable. Are you using git ship in real life, or are you merging pull requests via the GitHub/Bitbucket/GitLab/Gitea web UI?

[FFdhorkin]

I just found Git Town today. With the current behavior, I'd never use git ship

[bryanlarsen]

I used to do what you describe, but big PR's never got a good review. A 10 line PR will get constructive comments; big ones don't. And it was a giant PITA to keep reshuffling my commits into logical atomic commits. Telling my reviewers to review commit-by-commit sometimes helped, but frankly the hub reviewing apparatus isn't set up great for that.

Now with git-town, what would have previously been a commit is now a PR. Instead of 5 commits in a PR, I have 5 "nested feature branches" and 5 PR's. My PR's are small and atomic and get proper reviews.

[kevgo]

These days most developers use Git Town to create and sync feature branches and create proposals, merge using the GitHub UI or a merge queue, and then run git sync to remove shipped branches from their local workspace. I suggest you do the same. In its current form, git ship is somewhat soft-deprecated and really only useful for edge cases like offline development while on airplanes.