[Can we?] Disable merge commits to this repo and stick with linear history.

[gapisback]

HI, @gregthelaw -- I see that my recent PR to this repo has resulted in a merge-commit:

2e5fc6d gregthelaw 5 days ago Sun, 03-Mar-2024, 07:17:48 PM (Authored: Sun, 03-Mar-2024, 07:17:48 PM)
 Merge pull request #2 from undoio/gapisback/code-cleanup

aa3797c Aditya Gurajada 6 days ago Sun, 03-Mar-2024, 06:30:44 AM (Authored: Sat, 02-Mar-2024, 02:59:35 PM)
 Cleanup repo sources, tests, Apply pylint. Update README.md

From past experience, I have not found much value in merge-commits, especially the diffs of the merge-commit, 2e5fc6d, are identical to the original commit authored, aa3797c.

I think only you as a Owner has permission to change this thru Settings drop-down.

Going by this GitHub article, I prefer to have a linear history and squash all commits to a single coherent commit when pushing to /main.

What's your take? And can we finalize on something?

Thanks, for listening.

[gregthelaw]

I used to feel that way, but the advantage of the merge commit is that you can more clearly see the different chunks of work from the commit history, and they map conveniently to the GH pull requests. I found that once I got used to this way of working I found it good.

That said, it's not exactly a hill I'd die on. If it's super problematic for you we can change it.

[gapisback]

@gregthelaw , If I understand this right, what you are saying is that:

I, as a code developer, can produce a PR with 10 commits, each with some small change(s) I did while building the change-set. This gets reviewed, and may result in 2 more additional commits during the review cycle.

Then when the PR is pushed into /main, I will get 12 dev commits -plus- one merge commit?

Yes?

If so, I find that very distracting and unhelpful. If I look at git log output, I will see a trailing series of commit messages arising from my (private in my desk env) dev-work, which only have the value of retaining dev history thought process. Then, one additional merge commit to reflect this series of changes being pushed to main.

I have found that littering git log with small-small dev-commits is actually distracting. As a code author, I will have to have the discipline to put in good commit messages even for my incremental dev commits, which is a needless burden.

---

If there are no other scenarios where merge commits benefit other than this one, I would vote strongly against merge commits.

And think we should go to the model of one-clean-commit-with-clean-commit-message from one PR merged to /main.

From past experience with bigger dev projects: This tooling may be small now, but if it grows, it will be better to have linear history, with all dev-commits squashed-at-push time, to keep one commit.

[barisione]

The way we handle PRs and git history at Undo is that we consider each commit as an independent change which makes sense on its own (and, at least in theory, should pass all tests). When a PR is marked as ready for review, it's not supposed to contain any “uninteresting” commit just to fix typos, build problems, etc. This is achieve by extensive use of fixup and squash commit together with git rebase.

Once the PR is marked as ready, we don't rebase any more. Review comments are addressed via fixup/squash commits that get pushed to the branch. If there are such commits, before merging into master, the branch is rebase and fixup/squash commits are squashed.

We also never allow merges from master into other branches as those make the history complicated. Similarly, any conflict with master is resolved by rebasing, not merging.

The end result is that each branch merged into master is supposed to have a clear set of commits with useful content and commit messages.

[gapisback]

HI, @barisione, Marco, Good to e-Meet you.

The explanation makes sense. I agree with the dev methodology your outlined. And it's inline and consistent with one of the last things I also said in my prev post; i.e.,

"And think we should go to the model of one-clean-commit-with-clean-commit-message from one PR merged to /main."

---

It still leaves me puzzled as to why you need a merge-commit to get stuff from PR's dev-branch into /master?

For instance, please take a look at open PR #2 which I just submitted for review. It has code done exactly like you describe, with a single commit with just related changes.

Why would I not finally rebase v/s HEAD of /master and push into /master? Why do we still need a merge-commit?

The point of discussion / debate is the merge-commit.

[gapisback]

Oooops, @gregthelaw and @barisione : I had to push changes from PR #6 to /main and out of habit, I did "rebase and push" to main. Here's the commit log:

8283e76 Aditya P. Gurajada 3 days ago Wed, 13-Mar-2024, 03:37:55 PM (Authored: Sun, 10-Mar-2024, 04:47:01 PM)
 [Build] Rework Makefile rules to ease addition of LOC package

ae637d3 gregthelaw 3 days ago Sun, 10-Mar-2024, 07:59:02 PM (Authored: Sun, 10-Mar-2024, 07:59:02 PM)
 Merge pull request #5 from undoio/gapisback/reorg-tests-Makefile

867e3a7 Aditya P. Gurajada 5 days ago Sat, 09-Mar-2024, 08:32:13 PM (Authored: Fri, 08-Mar-2024, 04:43:43 PM)
 [Build-CI] Reorg code dirs. Separate l3 src/ from sample tests

Note that Greg's commit is a Merge-request, but my latest commit is just a rebase-and-push, retaining linear history.
Can we just keep this in for now?

This is the kind of pushes I was hoping we'd do to this repo.

Still ... awating ... a resolution on the items aired out on this thread. (Nothing pressing urgent; just a reminder).

[gregthelaw]

I'm happy to keep as is if @barisione is happy.