6 ms·
Atomic commits can be great. I really prefer to see intentional atomic commits with meaningful messages than 20+ files changed with a commit message: "Feature X
by mrinterweb 2y ago
Atomic commits can be great. I really prefer to see intentional atomic commits with meaningful messages than 20+ files changed with a commit message: "Feature X".
I am 100% onboard with devs squashing their "lint fix", "test fix", "whatever minor fix to get CI working", "generally meaningless commit to cross the finish line". Also, if devs are working on something they check in a "WIP" commit. It is great if you smash your WIPs into a single meaningful commit. These manual squash strategies require some discipline to clean things up.
I think the "squash branch to a single commit" merge strategy defeats the purpose of atomic commits. Of course devs will be bad at atomic commits if the commits will inevitably smashed to a single commit. IMO squashing branches on merge is a bad version control strategy. I love it when commits are intentional.
One rule I have for any rebasing is, when there may be more than one person using the branch, no more rebasing that branch.
- tkiolp4 2y ago> Atomic commits can be great. I really prefer to see intentional atomic commits with meaningful messages than 20+ files changed with a commit message: "Feature X". The most valuable “feature” of git for me is the “single commit with 20+ files changes”. I explain: I have landed in a new codebase, and now I need to add a new feature that would include perhaps a migration and adding a “usecase” and perhaps a “controller” and the corresponding tests. As usual, things are never clear in new codebases (depending on the library/framework used, one might need to add routes to a main routing file, or perhaps touch some configuration yaml file to whitelist the newly introduced endpoint, or perhaps a changelog needs to be kept updated whenever a migration is introduced, etc.). My point is: if I can git blame a line of code of the codebase that’s doing already more or less what I want to introduce, I want to see ALL the files that were touched as part of that change. It’s a lifesaver.
- deleted 2y ago[deleted]
- mrinterweb 2y ago> How many years have you worked in the software industry? Professionally, 21 years. Seems like you're implying that someone with an opinion that is different than your own is not experienced.
- tkiolp4 2y agoI deleted that part because it sounded the wrong way (it wasn’t my intention to question your experience, but a genuine curiosity)
- mrinterweb 2y agoGit has great tools. It sounds like you'd want to see everything associated with a merge commit. If what you're looking for is a certain way to view changes, git's built in tooling usually has you covered regardless of commit strategy. So if you know a commit sha and you'd like to see all of the merged branch changes associated with that commit as a single patch, I cooked up a script that takes a commit sha and finds the merge commit and show all the changes in a consolidated patch. #!/bin/sh merge_commit=$(git log -n1 --merges --pretty=format:"%H" $(git rev-list --ancestry-path $1..HEAD --merges -n1)) git show -m -p --stat --format="" $merge_commit
- parpfish 2y agoWhat do you do with the commits that are “incomplete”? I frequently get halfway through building something, but the. A more urgent project comes up and i need to commit my half-done non-functional work so I can hop onto a different branch
- erik_seaberg 2y agoI stash half-baked work to keep it separate from quality commits. When I come back to the stash, I may or may not like where it was going and decide to build on it.
- parpfish 2y agoStashing always makes me nervous. For some reason it feels ephemeral and I worry that I’ll lose the stashed changes like when you accidentally overwrote the copy/paste clipboard
- recursive 2y agoYes, I generally don't use it much, because it forces me to remember that I have stashed work.
- deredede 2y agoI used to feel this way because I lost work to `git stash pop` in the past, but there is now a nice feature where if you `git stash pop` and it doesn't cleanly applies, git keeps the stash and I use it more often now. That said for me the stash is usually used either for temporary stuff (eg taking my current work to another branch) or for things that might be useful to reference later but I will rewrite differently anyways; stuff I want to keep goes into a WIP commit.
- klyrs 2y agoCommitting everything to a new (temporary) branch, and returning to your current branch, costs nothing and saves the headache of stash's weirdness.
- 2y ago
- tmtvl 2y agoAtomic commits are also really nice for when some stealthy tricky bug appears and you need to git bisect to find out where it was introduced. Ending up with 'the bug got introduced somewhere in this massive commit that touches nearly every part of the system' is not very helpful.
- zikohh 2y agoI agree 100%. I also like using FF only. Reviewing commits is part of the code review process - their to title, description and code. If the commits are not atomic and clear I don't approve the merge request (MR); I believe the MR is a full snapshot of what will go to main. It's so frustrating to see 7 commits for one MR and it's a 10 line change.
- GauntletWizard 2y agoBranches should be small enough and short lived enough that they are atomic commits. Merge early, merge often, don't break things. Work on small pieces that don't change functionality, The missing part of this is that this causes you to need merge-trains earlier - You'll want to start "stacked" commits that are based on the pieces that are atomic but not merged. That sucks, because the tooling isn't yet great at handling that - It should be possible to mark and auto-rebase all your local branches in that dependency chain,
- account42 2y ago> You'll want to start "stacked" commits that are based on the pieces that are atomic but not merged. That sucks, because the tooling isn't yet great at handling that The tooling for that is called a branch with multiple commits and it works fine. The unit for something that makes sense as a single commit and what makes sense to review together is not the same and forcing the latter to be as small as the former will only lead to merging dead ends that are fine on their own but don't actually lead to the optimal goal.
- deredede 2y agoI generally prefer PRs that have multiple atomic commits to be split up in separate PRs. If the atomic commits can't be reviewed separately/introduce non-working states, they are probably too small. > One rule I have for any rebasing is, when there may be more than one person using the branch, no more rebasing that branch. That is the one rule to obey for rebasing to be useable at all.
- account42 2y ago> I generally prefer PRs that have multiple atomic commits to be split up in separate PRs. If the atomic commits can't be reviewed separately/introduce non-working states, they are probably too small. No commit should introduce a non-working state but it often still makes sense to review all commits that form a new feature together. After all, you are not just reviewing if every commit does what it says and doesn't break anything etc. but also that the commit is the correct approach for the final goal - and that often requires knowledge about how later parts are implemented.
- arwhatever 2y ago> One rule I have for any rebasing is, when there may be more than one person using the branch, no more rebasing that branch. Is it just me, or is it genuinely funny that whenever someone does this I think of Deep Thoughts by Jack Handey: “When you're going up the stairs and you take a step, kick the other leg up high behind you to keep people from following too close.”
- bluSCALE4 2y agoThat's a good rule. I need to follow it. My rule is, I'm a rebaser, I will destroy your remote so one of use should rename local branches.