[actinium226]

Just to be clear, and you probably won't disagree with this, there's nothing wrong with commits like "temp" or "checkpoint" or "WIP" (Work In Progress). I often make these sorts of commits as I'm working on stuff.

The issue is in submitting an MR/PR with those commits. There's an expectation among professionals that you make your work presentable before submitting it for review, although those who are new to the profession don't realize that this cleanup step is necessary (how could they? the intro courses don't teach this and they're usually struggling enough with the code).

I just wanted to throw this comment in here in case some newbie sees this and comes away thinking "oh, I can't have 'temp' or 'checkpoint' in my commit messages"

[taberiand]

I encourage developers to clean up their commits but so often they're either unduly nervous about breaking things or have a fascination with "preserving the history" (of their branch, which is not yet merged in anywhere).

I partly blame the excessive fear mongering around rebasing, where the strict Never Rebase a Pushed Branch rule is drilled into them and they never learn why or when they can break the rule safely.

So it's an uphill fight but I just try to teach by demonstrating, frequently, exactly how they can tidy up for the merge request.

[magicalhippo]

> I just try to teach by demonstrating, frequently, exactly how they can tidy up for the merge request.

Some recommended resources for this?

[hyperbolablabla]

I couldn't care less about the commit messages on a PR, I care about the diff. as long as the messages are cleaned up in the squash and merge with main

[OJFord]

If it's large I want to be able to look at the commits in isolation and understand the work in the logical chunks that made sense to the author.

If what made sense to them is temp, checkpoint, temp, temp, undo the temp, fix test, try this, that didn't work try other thing, maybe?, temp, fix test - then I don't stand a chance. Recently I reviewed one that had multiple 'rebase' commits, I have no idea.

[scott_w]

> If it's large

Then make the PR small.

[keybored]

This is a conversation that keep repeating

- Many temp commits

- Doesn’t matter: just squash

- But sometimes I want to have a few distinct (isolated commits for my PR)

- Then make the PR small

- But I have several changes

- Then make several small PRs

We keep coming back to “just make more PRs”. Which is curious, given that GitHub doesn’t even support dependent PRs. The thing you need immediately when your PRs start depending on each other (like refactor X before implementing Y, where both touch the same code).

But I can’t come to any other conclusion than that this is because of the over-focus on PR as the one and only reviewable unit. Thanks to GitHub.

I can’t really get on the small PR train. A PR has overhead and I often want to do small changes that I only happen to notice are necessary when I am in the middle of doing that one PR.

[scott_w]

Don’t make dependent PRs. Make small PRs, review, merge and deploy them before starting the next.

[keybored]

That begs other questions. The relevant reviewer isn’t available until Wednesday. Do I start on the next tasks in dependent branches? The act of making all those branches is a large part of the inherent busywork. And then I need to remember to get back to them when the reviewer gets back.

The benefit of all these small PRs is yet to be revealed.

[OJFord]

1. I'm reading not writing.

2. Is our 'large' the same?

3. I'd rather the PR were whatever size it needs to be to entirely do the thing it's supposed to do (and nothing else) than conform to some arbitrary size requirement.

[scott_w]

You're over-thinking it. Large PRs have a known effect of reviewers' eyes just glaze over and the PR just gets a cursory glance and LGTM :+1: on it.

That's when you know a PR is "large."

What's stopping you breaking such a PR into smaller chunks? Some arbitrary "it does what it's supposed to do" definition?

[Cthulhu_]

But that's the thing, in most cases there's no review step anymore after the squash/merge has been made; while it's in a branch / MR, you can still edit the commit messages and content, but in most cases the squash and merge is an atomic, unreviewed step. Of course, maybe the merge commit should be generated, or should be filled in and reviewed as part of the main code review.

[mkesper]

Why is squash and merge a trend now? This destroys all advantages of git in my view.

[keybored]

Yeah, I’ve noticed that many around the Internet just comment with “the squash and merge” as if it’s a given that that’s the strategy that is used (even mandated).