Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

As someone who is a big proponent of "squash and merge" I am susprised to see myself labeled an extreme clean freak! I don't have very extreme git uses or opinions.

I want a few things in my team's workflow:

* A commit history that a human can mostly understand

* Effective code reviews where I can easily see what changed between rounds

* The ability to revert a whole Pull Request easily and cleanly

* A local workflow that does not get in my way and allows me to be messy when I need to be

For our team that means we commit whatever we want to our local branch and then squash and merge the whole thing at the end. The result is a single commit saying "Adds feature foo". I don't feel like I lost anything in the squash ... am I missing something? I really don't want to have "fix lint, address review comments" in the shared commit history.



> The result is a single commit saying "Adds feature foo". I don't feel like I lost anything in the squash ... am I missing something? I really don't want to have "fix lint, address review comments" in the shared commit history.

Neither.

If you're doing it then way it's done in Linux, you will end up with neither of those. Instead you will have: "Add a new geometric engine with tests", "Add the geometric engine to view API keeping backward compat", "Use the new view API extensions in the main window renderer", "Implement behaviours for feature foo in the renderer", "Hook up scripting extensions, keys and menu options to make feature foo user-accessible", "Add basic documentation for feature foo".

No "lint", "reviewer feedback", "fix typos" or "I changed my mind and reverted the last revert" trivia goes in the history.

You should not in general submit your minute by minute trivial actions, indeed the commits submitted are usually not even in the same order as they were written and refined

Nor are the above individually complex, logical, meaningfully revertable and bisectable changes squashed down to an unhelpfully crude one-liner "Add feature foo" with a 5,000 line diff that touches most files in the project.

Some would say the logical changes should be separate PRs. To some extent, but it depends how fast you wrote the feature, whether the round trips make sense, and how involved in the nitty gritty you want the feedback to be.

The ideal unit of review or feedback, ie PR size, is often larger than the unit of a logical change. That's because the "future why" is not so apparent with individual changes, so people tend to zoom into the weeds instead of using appropriate context. In so doing they also tend to use up their reviewer attention budget so the other changes don't get the same scrutiny.

Think about the feedback to the first big logical change "Add a new geometric engine with tests": "Why are you adding this? What is the motivation?", or "This works but the API has these unnecessary warts" (warts you have to explain will be necessary in 3 PRs time). You can either have a chat in the PR feedback where you have to persuade someone that the rest of your plan makes sense, not easy if they can't see how it does. Or you can present the later changes all together as a group and then the code speaks for itself, much more convincingly.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: