5 ms·
Squash and rebase are doing it wrong.
by RockingGoodNite 4y ago
Squash and rebase are doing it wrong.
- dtech 4y agowhy do you care about my 10 "wip" commits on a feature branch? Why am I not allowed to package a change in a nice set of commits once it's done/reviewable?
- rovr138 4y agoNot them, but I would hope the messages are better than "WIP" and expose context vs a commit that simply says 'Implemented X feature". I usually leave those for a tag.
- kjeetgill 4y agoIf you don't have commits called WIP you're not committing enough. You're doing it wrong. I kid, I kid! But I do commit just to push a back up before getting on a train, or a convenient point to diff against as part of a refactor, etc. They should all be a single commit by the end.
- rovr138 4y agoI think usually my issue is with context. I know the benefit of squashing, but I feel they get abused and you loose a lot of context. WIP of a function/method, they could be squashed into a good commit that explains what and why. But some people implement a full feature, squash all of it into a single commit that simply says 'implemented Y'. But you loose context on why they modified the different functions/methods. Yes, you can try to find it, but there's no context so you're just dealing with a huge block of code that modifies other things, doesn't just add new code.
- ajross 4y agoI second the "you're doing it wrong" note. The use of a tool that permits easy squashing and rebasing opens up new ways of doing development where you can commit and checkpoint at any moment, cheaply. So e.g. when you inevitably break something, you can bisect to where it happened, or when you find you've added a mismatched assumption you can see where the thought process went wrong. You don't need to have things tidy, you know you can recover anything you do. So likewise when you're "done" you can squash and split them into changes that make more sense logically before pushing to some shared tree that someone else is going to have to reason about. People who develop without tools like this tend to do it in a big flat directory and think about "commits" as something done every day or so. Once you get beyond that style, it feels really clumsy.
- int_19h 4y agoI think that preferences along these lines are inherently subjective, and some people may well really be better served by the more traditional workflow where every commit is something you commit to. I just wish this wasn't presented as the one and only right way to do things, to the point where software actively resists any other.
- rovr138 4y ago> So likewise when you're "done" you can squash and split them into changes that make more sense logically before pushing to some shared tree that someone else is going to have to reason about. Which keeps context which is my point. The other case that I see is squashing it all into one that simply says 'Implemented feature Y' which doesn't provide any context into why something was changed.
- dolmen 4y ago--amend
- bayindirh 4y agoYou can't --amend remote, sorry. If you don't backup your code to remote even if it's not finished, you're taking serious risk.
- cestith 4y agoWell... you can force push to a remote with sufficient privs, but you really don't want to do that if you can help it.
- bayindirh 4y agoWhy should I abuse git while I can commit my developments part by part with nice descriptions?
- sshine 4y agoBecause you may want to back up your work remotely before you have the time to write something nice. I had to leave halfway through writing a unit test today. My colleague asked me to back up the code by pushing it. And now I'm back, fixing that test, writing that commit message. Most likely we'll merge right after, so my colleague probably won't need to add more, so the branch is in my control.
- sshine 4y agoA remote can be your personal remote endpoint, or it can be on a shared remote endpoint, but prefixed with your username. As long as you `--force-with-lease` to a feature branch you control and agree on a protocol with potential collaborators, the harm is minimal. I mostly avoid creating these kinds of conflicts when people's limited git experience would cause stress or unnecessary use of people's time fiddling with the history. Or if more than one person is doing stuff independent of one another. I've had colleagues that frown on rebase, and people who can't not. One of the beauties (and complexitties) of git is that it allows for this diversity of workflow.
- sanitycheck 4y agoI like seeing small commits, mine and other peoples. It means when something's broken I can easily narrow down the cause to a single small change and either unbreak it myself or point the appropriate team/person at it, making it a quick and easy fix. If squashing happens then all I can immediately say is "something in this 600 line changeset over 12 files for Feature X broke it". The bug report for that one is going to be more vague, get allocated more story points, and maybe stay in the backlog for several sprints (or forever). If people are pushing their feature branches and not deleting them that makes life a bit better, but generally people who want their git history to be "clean" and squash commits also want to get rid of old branches. Someone might say code review should have caught it. Code review almost never catches actual bugs. Or tests? Tests only test stuff that the author expected to break. The main things I try to ensure are that every commit is small, every commit has a useful message, and no commits should break the build.
- int_19h 4y agoSquashing and rebasing doesn't have to produce one gigantic commit. It can take, say, 20 "WIP" commits (many of which might not even build), and organize them into 5 commits that constitute logical units with descriptive messages. But raw WIP commits themselves are rarely so organized, unless you invest a lot of time and effort into that while coding. Personally, I prefer my coding to be free of such distractions, and to use SCM facilities as a scratchpad for fast iteration on the code (the ability to easily reverse changes etc); this results in many commits that don't make sense in the final pull request / code review.
- sanitycheck 4y agoI've heard of people doing this, but never seen it in real life. Do each of your "logical unit" commits build and run properly on their own? I'm interested in the concept, if there's a tool/process which makes that easy to achieve - but it sounds like possibly more work than just making good small commits to begin with.
- int_19h 4y ago
- craggyjaggy 4y agoWhat message do you use for the commit you make after finishing for the day? Me going home usually does not coincide with any feature or change being completed.
- rovr138 4y agoCompleted function <A>. Started function <B> Code for function <B> has the skeleton. Still need to add <...>
- bayindirh 4y agoI generally work on a single feature over many commits. Sometimes I need to leave the thing half finished, so commit as is with a nice commit message detailing the state. At other time I make "transfer commits" since I'll continue development on another machine and need the latest snapshot of the code to continue. Not all features fit into a single new function, we need to move mountains to rearrange stuff, and to keep code tidy. As long as the commit messages are clear, and the code works at the end, it's alright.
- rovr138 4y ago>Not all features fit into a single new function, we need to move mountains to rearrange stuff, and to keep code tidy. Of course. But when modifying another, you can commit and say, Implemented function X. Modified Y to accommodate this new argument that's used on X... and so on >As long as the commit messages are clear, and the code works at the end, it's alright. I'm fine if the commit messages are clear. The problem is when squashing, some people squash and don't keep commit them clear or some even squash into one that doesn't provide any insight or clarity, 'Implemented feature Y' and you get a diff of thousands of lines that touches everything
- bayindirh 4y ago> Implemented function X. Modified Y to accommodate this new argument that's used on X... and so on Sometimes I need to write "Implemented function X, but evaluates the result wrong possibly because of this. Fix this first, then continue". > The problem is when squashing, some people squash... A single commit touching whole codebase and only says "Bug fix" (or similar) is bad. I concur. Also commit history should have enough granularity allowing bisection and partial rewind to understand problems and other side effects.
- pizza234 4y ago> why do you care about my 10 "wip" commits on a feature branch? If that's how a team develops PRs, then the commits are structurally suboptimal, and the problem is in the development practices. Such team won't be able to efficiently bisect, independently of the repository structure. I personally work with granular, self-standing commits, and with this workflow, non-squashed commits makes sense. Obviously it's not possible to always have self-standing commits, but it is possible the vast majority of the times. It takes a lot of practice and discipline, though, and if a team is not willing to put them, then of course, squashing in the only way that makes sense.
- samus 4y agoWhy would I want to bisect on a feature branch? Edit: I strive to keep my feature branches both short (# of commits) and light (# of lines changed).
- pizza234 4y agoAfter a feature branch is merged, its content is candidate for any bisect. If the branch was squashed, bisecting will be less effective, compared to the same branch, merged without squashing (at the conditions of the commits being self-standing).
- MichaelCollins 4y agoDo you find that you often have authoritarian takes, or just in matters pertaining to software development? If the latter, I suggest you pause and reflect on that. There is no one right way to do things, different workflows work for different people. Tools which accommodate the most number of people will be the most popular.