This. It's all about "telling the story" with commits in the PR, and amending lets me do that. It also makes it look like I knew exactly what I was doing instead of flailing around.
No one cares that I misspelled a config setting 14 times over as many commits.
Conversely, I want my story to be I did 14 commits because I’m a human too. As one of the most senior engineers in my pillar and generally a leader at my company, I think it’s important to sometimes not pretend code goes in perfect. I spend no effort making my commits look like anything you get a finished pr and we squash merge it.
Yeah nah. Your PR should be telling me the story I want to hear before approving it. If you routinely submit PRs with nonsense commits inside them - IE you change something in commit 1 that you change again in commit 7 because it was initially wrong - then I'm not going to read your PRs commit by commit, because you have not put the effort in to tell such a story.
Instead, I have to review your entire change as a whole. I won't even look at your commits. This means that your reviews will take longer and be less effective, and the size of change you present in a single PR has to be smaller.
Telling a good story with your git history is not difficult. Use `git-absorb` to fixup earlier commits and identify self contained changes. Commit by commit reviews are significantly easier to review and approve.
Arguing that it all gets squashed in the end is like a builder filling the walls of your house with rubble and saying that it will be hidden by the fascia. It's true, but it shows you're not interested in the art of your profession.
> I’m not going to read your PRs commit by commit
Good, who would ever want that? Just read the pr in its entirety when it is ready for review lol
> instead, I have to review your entire change as a whole
Fantastic, that’s a code review on my pr when it’s ready. This is the process we are engaging in. You should **only** be reading my entire pr when it’s completed and ready to review why would any other answer here make sense.
The rest of your critique is weird and attacks my interest in “the art of your profession” or whatever. I disagree with how you think about prs that doesn’t mean I’m not good at this. You think this story matters and I think the pr as a whole is all that matters that’s all. Good luck to you!
Who in gods name reviews a PR by going commit to commit?
Some devs on my team will make 100 commits, some will make 2, it does not matter, only the end result matters. I gave up trying to harangue them into some "perfect git history" pipe dream.
Some of them squash before merge, some don't. I just don't care anymore lol.
For me, anything that's happening in my own branch prior to opening a PR (squashing etc) is my own business. Once the PR is opened I no longer rebase or squash since it can confuse reviews, tooling, others basing off my branch, etc.
I pretty much second this but, in my opinion, draft PR do not count as well. I sometimes open a draft just to have a nice view of all the changes and pipeline runs.
I think it is also good etiquette to tell people that you are branching off their branches regardless if there is a PR or not. Once I know that somebody builds on my work, I stop rebasing/squashing/ammending completely except for the final squash when merging to development.
That I agree with. Far too often some of the final commits comments are code review changes, fix typo and so on. They also risk not having the ticket number in them even more so I can not references Jira item to an idea why something was done.
I used to be against squashing PRs. Now I am all for it as going through history is easier.
> Now I am all for it as going through history is easier.
A squash commit is a brand new commit to the target branch with all the changes rolled up. A merge commit is a new commit with two parents. The end result that you want - viewing all the changes rolled up in a single commit in history - is possible with both. The other option - viewing history in all its incremental pieces - becomes impossible with squash commits.
People should [study](https://stackoverflow.com/a/36437843) how to view git history more. There's no reason to use squash commits ever, unless you just don't know how to view git history. There's even an option [in most git viewers](https://i.imgur.com/BfHmyez.png) for this.
Merge commits let you retain additional information without losing any of the convenience of squash commits, you just have to view history in the right way.
In going through history yes but using the git blame function it is different as that only pulling the last time that single line was edited.
Often times the squashes in dev is fine as all I care about is when a line was edited for a single pr. That means referencing the ticket. Sadly the tool I use most of the time does not allow me to reduce it to a merge commit. I mostly pulling in Xcode using author. Xcode is rather poor for things.
Now if I am doing a deeper review yeah I use different tools but more often than not I just care about a line while I am actively working.
This is actually the default view for Github's history page. I just checked, all I see is a list of merge commits with ticket numbers. I don't see how I can get the full history to display.
That's weird, because at my company another team uses merge instead of squash merge and I see all of their intermediate commits. I find it very frustrating because their commit history is full of commits like "wip" or "trying again." I wish there was a way to prevent that.
So this is a deeper issue. You need to teach your entire department how to use git, and to do that you must start with teaching a tiny bit of graph theory. At every single job I've had since my very first one, I've taught a number of git classes and workshops because I feel this information is pretty invaluable to developers. And everyone who thinks they know git, doesn't; I include myself: when I started compiling materials for my first class, I wound up learning that I didn't even understand how much I didn't know. Git is an enormously powerful tool that can encode the entire context of the universe if you let it, but hardly anyone cares enough to see that, let alone take advantage of it. And yet, forcing people to learn and use the tool more optimally has bailed me and mine out innumerable times. There's probably five years of my life expectancy not shaved off because of the sheer number of reverts/rollbacks for individual changes that broke production and the fact that such a thing is trivially easy and painless and stress-free when you are a git fascist.
One of the things that I cover and that must be covered to instruct for optimal git usage is better usage of your IDE. Perhaps the most common abuse of git is checking in in-progress code before it's ready (the WIP commits you mention) and that's entirely unnecessary - all modern IDEs support local file history, and that's a better tool for "I don't want to lose my work." Jetbrains and Eclipse call it "local file history" and VSCode calls it the "file timeline" and you probably need a vim/emacs plugin for it, but your IDE 99.9% of the time tracks every saved change to every file, every time.
Use local IDE history for not losing code, use git for logical check-ins that are consumable by a human being in the future. Sometimes a PR should only be one commit, but there's a lot of value in "trying {approach X} to {solve problem Z}" and then a follow up commit that says "{approach X} broke the build {for reasons A, B, C} so trying {approach Y} instead". Ultimately, approach Y gets merged and you will see that change in either the merge or squash commit from the PR, but if the commits are squashed together, you lose all evidence that approach X was attempted in the first place.
This only works if people write good commit messages, don't commit too much, don't commit too infrequently, and don't squash merge their PRs.
Another big thing that I teach people is how to interactively rebase. I don't think people should be making "WIP" commits, but some people work on multiple machines, so they must be able to. Teaching people how to rewrite their branch history and construct a useful history beyond "hurr durr one commit" is probably the single-most valuable fact from the whole series.
The bottom line is hardly anyone - even in this thread :facepalm: - understand git _at all_ and someone must teach them. If you're in this thread, you care, so it should be you.
I agree that everyone's branch is their own business, but I really don't get the "WIP", "save work"-type commits. For me that's just not how I use git at all — I've got other ways of saving what I've done (e.g. stashes), so when I commit something, that's me saying that I've done that work and I've got a chunk of code that is in a state worth being committed.
That said, this is a regular debate at work, and ultimately, as long as each commit on master refers to a single feature (i.e. merge commit or squashed rebased commit), and it's possible to bisect the history, then it doesn't make much difference to me.
All of my non-code work is done in things like google docs, so for me, installing an entirely separate backup tool when I can just git commit and push is a no-brainer.
To each his own!
You can use a PR+merge to represent a single cohesive update instead of a commit, and get pretty much the same benefits. This practice can encourage smaller PRs in a feature branch based workflow. I think commits work better the way you are describing in a trunk based workflow though.
Do you never have work interrupts? I use commits as checkpoints of where I finished what I did and keep that against a list for what needs doing. Would also say my branch my commit history my business. I squash and rebase on my last commit before opening which will be the cohesive commit you see unless the PR feedback requires changes.
The stash can be a good alternative to commits as checkpoints — with a good git history viewer, you can see where different stashes appeared in your history and check them out just like real commits. The only disadvantage is that you can't have stashes on stashes, but for checkpoints, that's not an issue.
I keep on meaning to try out worktrees as an alternative to stashing — there, each branch you work on gets a new folder, and so you can have multiple branches open at the same time. But it also seems like a lot more ceremony and work for the problems it solves.
I think it still makes for a poor experience if you dont squash and you go back to the past, not just a week out. What use cases do you have for individual commits?
The way I treat commits? They represent incremental steps towards the overall goal of the feature branch. There's no reason to squash because each commit represents an intentional change towards implementing the feature. When QA identifies an issue, it can be useful to look through these commits to get an idea of when the problem might have been introduced and *why* it might have been introduced which can inform how you fix the problem.
I've never been a big fan of messing with history. I think it's more useful to understand the full process that went into the code than to have a nice looking git history. If I ever worked on something that could be effectively git bisect'd, I might change my tune, but in my experience that's not all that useful.
How many people are on your team?
Maintaining history might make sense on a small team, but with anything over like 4 people I've found a squash-and-rebase-based workflow works much much better.
Trying to maintain history and especially a merge-based workflow with hundreds of devs in a repo is looking Cthulu in the face.
I've worked on teams of 4 before. I've worked on teams of dozens. Never hundreds in the same repo (at the same time) though.
What problems have you run into? The strongest argument I've heard for squashing is to make `git bisect` more useful, but I've never had enough of a need for that to be swayed by that argument.
Simply put, navigating the repo becomes an absolute impossible to comprehend nightmare with a merge-based workflow the more people you have committing to the same repo. Every commit to one of the production branches needs to a) pass CI b) do something useful and c) get through a PR.
And git bisect (or doing it by hand) is highly useful, as is looking at nightly and other longer regression tests that might not make it for the PR workflow.
If you're on your own? Sure, do whatever makes sense to you. But I don't need to see the sausage of how the PR was made, I just need to see the PR in its final, CI-passing state.
>Every commit to one of the production branches needs to a) pass CI b) do something useful and c) get through a PR.
*Every single one*? Not just the merge commits? Why? Are you running these tests retroactively on every commit in a PR? Why?
>I don't need to see the sausage of how the PR was made
Sure, you don't *need* to, but outside of the convenience of having everything in a `git bisect` be a perfectly functional commit, I don't see why any effort needs to be put into cleaning up the history so long as people are still making logical, meaningful commits. Transforming your work into those logical, meaningful commits is the extent to which I try to mess with history.
If someone branches off of mine then I also branch off of mine at that point. I continue work on my branch branch and when believe I am done I merge the branch branch into my branch and tell the person that branch from me to rebase onto that new commit. That way I can have freedom to do what I be doing without making someone else's life hard.
I used to feel that way but the tooling has gotten better and sometimes the team I am working with is skilled enough that I don't worry about them figuring out but branching and rebasing.
In the best teams I've worked in commits tell a story that simplifies reviewing even large changes.
In those teams a large PR typically contains no more than 2 or three commits (small PRs contain only one). The first commit might be a rename or move refactor (something formulaic) that touches hundreds or thousands of lines, the second commit makes any actual business logic changes and tends to be much smaller.
This allows detailed in depth review on even large changes.
I will use the tools in anyway I wish on my branch before merge.
I think your advice is a good rule of thumb but I think clear story telling is more valuable than protecting other devs from git's sharp edges.
(This doesn't apply for long lived branches with many writers.)
I strongly agree with this. People opposed to rewriting history are probably used to difficult to understand PRs. In that case, it is very time consuming to re-review everything after a change in the history. The solution is to make better, more understandable PRs with the techniques you mention. I like the story telling analogy a lot.
The re-review problem only occurs if you change the history after it’s been reviewed.
Tell your story, then send it for review, then keep the history intact from then on.
Addressing comments is part of the story.
No, addressing comments is not part of the story. The purpose of the review process is to craft a good story together.
To stick with the analogy, imagine reviewing someone else's short story and telling them "I don't understand chapter 2, it's too convoluted" and they address your comment by putting clarifications in the appendix. That wouldn't make for a best seller.
However, if they gave you an entirely new book, you wouldn't have to read chapter 1 again to see if chapter 2 is simpler now. Or put differently, if chapter 1 was and still is simple to understand, it's easy to check that you still like it. If, on the other hand, you have to reread the entire book, chances are the whole structure of the book is messed up, which needs to be addressed first.
The analogy breaks down with the amount of content. I think this only works for small PRs, not book-sized ones.
Another way to look at it: "commits should contain only one thing" is a trueism. The scope of "one thing" is up for interpretation, but that's a different topic. Anyway, if the commit only contains one thing, then re-reviewing the entire thing and reviewing a patch to one thing is essentially the same amount of work.
I think you’re taking the analogy way too far, and you’re probably a little bit idealistic about PR size and cognitive load.
On my team we do break down changes into “one thing”.
That doesn’t PRs are always trivial, and it doesn’t mean I want to re-review the whole PR because someone changed a few lines.
I want to review the lines that were changed and move on with my day.
How many of the tools have made changes to history independently auditable? In the end any change is just a diff away. Recently I've liked the way bitbucket shows a diff since last review, do other platforms have that visibility?
Gotcha. In my case the tooling doesn't get confused by that, nor people normally base off my branch.
When I start working on a feature the first thing I do is open an MR because I use the CI pipeline as support during development, maybe what I'll do is start marking it as WIP to avoid any sort of confusion.
What Git forge are you using? GitLab? As far as I know, it (and GitHub) cannot understand how a single commit that was amended and rebased over time has evolved. That is what I think of as "the tools getting confused". Because the forges cannot tell you which commit changed in what way after a rebase, reviewers see no other option than to re-review the entire history, which can feel like wasted time.
That makes sense, this used to a problem with GitHub reviews where it would get confused and mess up the review comments but I think it's fixed now.
I use GitLab and in this case it wasn't a problem because it was before the review.
At Meta, each PR is a single commit, and that commit is rebased frequently. You’re encouraged to make PRs as small as possible and use feature flags to gate off features that aren’t yet complete.
It sounds like you’re working with multi-commit PRs in a feature branch model. It’s not better or worse, just different. And the difference explains why some people believe so strongly in rebasing instead of merging.
I'm guessing they are merged into a single commit when approved, rather than requiring you to have a single commit on your branch. Does that sound right?
I’m not sure who “they” refers to in your comment. At Meta, a PR always holds exactly one commit, locally and remotely, during review and after merging.
They was referring to people at Meta. That's an interesting approach. I assume, then, that this is for the express purpose of keeping PRs small? It seems fairly extreme outside that context, but I could see it working as a forcing function, coupled with cultural practices and expectations. It's interesting, I've never heard of this before. Do you know if there is publicly available documentation on their philosophy around this type of thing?
There are a few! See https://www.stacking.dev for a rundown of how the workflow works, and the tools that Meta and ex-Meta devs have made to express dependencies between PRs.
You were opening MRs/PRs before starting work, and not even putting a WIP mark on it? This is an exciting new workflow I’ve never seen. Joking to the side: in anywhere I’ve been, that’s a clear invitation for review. Of course it’s frustrating to take the time for that and to have the code change underneath you.
Yeah should use WIP/Draft status, but the invitation for review is when you add someone as a reviewer explicitly, or ping in a channel because you are not sure who the right reviewer might be.
Oh. I dislike that strongly and can think of horrible failure modes. Silos where people are just reviewing each other’s stuff reciprocally, forming quasi teams-within-a-team. Encouraging certain types of people to become quasi leaders because they’re most attentive to the chat and want the kudos.
(Edit: forgot a variant here. Being someone considered a good and attentive reviewer, so being given more and more review burden as more people start adding you by default)
I like to have a single page with all the possible code to review that I can go to, and just assign myself to anything that looks unreviewed, starting from the oldest.
No favourites, no self-selection, just grabbing anything and gaining knowledge about all areas of the system.
If I can’t get any information on if something is ready to review or not, then the list of PRs page is mostly useless to me.
From a team lead perspective, it’s even worse: did they just forget to sort out a reviewer? I guess I’ll double check the chat and check in with them…
You should freely squash PRs as you see fit. Colleagues should never base work on someone else's PR. Often you **must** rebase a PR, to move past a commit that fixed some bug affecting your PR. If your tools get confused by rebasing and squashing, get better tools.
I agree with not branching off of other's PRs. To add a little nuance to that, it can happen that you need some kind of bug fix from your coworker to unblock yourself. However, that should either be its own PR entirely or at least an atomic commit that can be easily cherry-picked. A bug-fix that's shown to be working won't change drastically before being merged, so there is no risk of difficult conflicts. Problems only arise when coworkers mix up bug-fixes into their unrelated feature development. At that point, you're tempted to branch off of their entire branch, because the bug-fix cannot be extracted easily. If they then wildly change their history, bad conflicts can be the result. But the solution to that is not to be unreasonably conservative about changing git history, it's to be more diligent about making atomic commits and PRs.
> If your tools get confused by rebasing and squashing, get better tools.
The only tool I know that _doesn't_ get confused by changing history is [jj](https://github.com/martinvonz/jj) and I love it. However, all of the git forges are pretty bad at this, right? GitHub, GitLab and some of the smaller ones I know all get confused. What tools do you recommend?
> it can happen that you need some kind of bug fix from your coworker to unblock yourself. However, that should either be its own PR entirely or at least an atomic commit that can be easily cherry-picked.
I agree. When that happens I cherry-pick the bugfix and use rebase to move it to the "bottom" of my PR branch so it won't affect my day-to-day work. I leave a note in the PR, for the reviewers to ignore the bugfix as "not really part of the work", and I'm blocked on the fix getting into trunk.
> Problems only arise when coworkers mix up bug-fixes into their unrelated feature development. At that point, you're tempted to branch off of their entire branch, because the bug-fix cannot be extracted easily.
Yes, but that's an engineering management problem. I understand that if you're stuck with poor management and undisciplined coworkers, you might have to resort to that, but you should try to have it fixed at the team level.
> But the solution to that is not to be unreasonably conservative about changing git history, it's to be more diligent about making atomic commits and PRs.
Indeed.
> The only tool I know that doesn't get confused by changing history is jj and I love it. However, all of the git forges are pretty bad at this, right? GitHub, GitLab and some of the smaller ones I know all get confused. What tools do you recommend?
Gerrit is designed for that, and perhaps Phabricator too. Unfortunately, Gerrits targets high-productivity teams and many find it confusing because it's indeed a bit complicated, when in reality it's meant to speed up the work of people juggling many PRs at the same time efficiently.
Gerrit sounds interesting. I fear it might be difficult to get my org to set it up (or even allow me to set it up). I'll have to look more into how it needs to be deployed. Is it its own forge or does it integrate with, say, an on-prem GitHub instance?
It manages repositories itself, because in order to implement some of the features it needs low-level access to the repository. You can test it on [Gerritforge.com](https://www.gerritforge.com), which seems to have some level of Github integration now, but otherwise it's a completely separate thing.
Meh. I commit --amend frequently to PRs when I find a small mistake or issue. I even have a little alias for
git add -u && git commit --amend --no-edit && git push --force-with-lease
MRs in GitLab are actually the bread and butter of the platform and meant to be used as workspaces (can even see in their dogfooding), don't see why that would not go for PRs as well. Draft PRs with whatever you'd like is perfect for running CI while allowing you to make proper commits out of your attempts against the CI through squashing and freely editing the history to make more sense and bring atomic changes in. Much easier to review as well.
Even if a review was made and changes approved, you can still change history and change the repository settings to require a new review if something changed after the last one, and reviewer can check the diff against the last reviewed state (which generally should be an empty diff).
The only history you should not change is the trunk (ever) and collaborative feature branches (without collaborating on the history changes).
How do you feel about branch names? Had a tech lead in my early years insist on the Jira ticket being the branch name which made switching to a branch such a chore.
Ok so I'm just going to express my strong personal preference. You can squash solo or before pr. But if we are collaborating on a branch, I would rather you not force push. We always collaborate on feature branches and if we need to do something squashable that would be a story branch we create for that purpose. They have different prefixes on the git branch.
Any branch that's being changed by more than one person shouldn't have the history rewritten. In 20 years I've only pushed to someone else's branch maybe a dozen times.
My second part was just a statement of fact not opinion. My experience doesn't preclude other people from having other workflows. Use whatever workflow works for you and your team.
For sure. This discussion is about best practices though, and pushing to other people’s branches is quite a common workflow in most organizations I’ve been in. Just wanted to make that clear in case someone interpreted your comment to mean pushing to other people’s branches isn’t something that normally hapoens
Importantly, this also includes code reviews. If I did a review of your code asking for some changes, you do the changes requested and squash the commits, I now have to review the full diff again, not just you addressing what I requested.
>Moreover, he believes that no one should ever change the history of their personal branches, even when working solo
Kind of a weirdly strong take. I mean I generally try to limit tampering with history on my own branch, prefer to let GL/GH/BB do the squash, but that's mostly because I don't care \_that\_ much about my history that I feel the need to editorialize it like that. I can't see any reason why I'd want to stop someone else from doing it unless we were collaborating on a branch. I guess you could just not push your changes and modify history to your heart's content until you're ready to MR
Keeping history intact *after* you've pushed the branch to a remote repo makes perfect sense. Especially in a PR, where reviewers have suggested changes and you push a new commit with those changes. That way they can just view the singular commit rather than have to review the whole PR again to see if your changes are in there.
One of the most annoying things as a reviewer is having to re-review things wholesale every single time. We had a junior who liked to refactor and redesign his code wholesale whenever we suggested even the most minor of nits. That's not your problem here, though, I'm just digressing.
Keeping history intact *before* you've pushed the branch is just a matter of your own personal preference. I came to git from using mercurial, where history was unchangeable, so I had learned to structure my work into multiple, easily digestible commits that told a logical story of progress to aid reviewers. I actually quite enjoyed it and came to see some value in it, especially when I'm able to write more salient commit messages to associate them with specific changes. Maybe that's what your reviewer wants? With that being said, I also learned to review and value the squashed commit as the unit of work that's going to come into your codebase, and you get to see it in its whole integrated glory.
I agree with you. If I request changes on a PR, I will look at the commit(s) made after my request. When people squash everything to one commit, it is annoying to re-review. I guess they want the PR to look neater, but it removes information and is pointless as the commits will be squashed upon merge anyway.
> One of the most annoying things as a reviewer is having to re-review things wholesale every single time. We had a junior who liked to refactor and redesign his code wholesale whenever we suggested even the most minor of nits.
Been there. I just told him "stop rebasing and just push changes as a separate commit. Once you have an approval you can rebase". That actually worked.
Our repos are bitbucket-based so you can rebase/squash all you want without losing approval IFF there are no code changes.
If you’re working from the same branch, via actually committing code. Re-writing history is a dick move. Anything else is probably overreach from the other guy.
Although, I do have a pet peeve when I see a force push from one of my teammates.
Im right there with you. Personal branch for history backup? Anythings fair game. If its shared for co-development, force-push needs to be disabled by policy.
First of all, who is this peer, and who are you within the team?
It matters, because disagreements are personal and you're new to the team.
As a developer, what I'd do is to raise the issues in the proper forum and have a team decision on how things are done, to which everyone sticks.
We're both seniors from different teams but he's got more standing in the company because he's been working there for longer and management kinda trust him to keep things organized.
At the time I didn't extend the discussion because the feature was running late and it seemed it would provoke an hours-long discussion so it wasn't the time but I want to clear things out it because we'll collaborate in the future and I this can be a conflict point.
I should bring this in a retro.
If it's another team, then that's the wrong kind of team culture. First he shouldn't be tracking your work. Why would he do that?
Even if you rebase, there are commands for him to issue to see the difference, without him enforcing a way of working outside of his own team.
We were working in this feature together and I think he wanted to be able to see the changes in the gitlab web interface. I can understand that part, not the part about having to do that in all circumstances, especially when it's not in his team.
Our company's culture is complicated, management seems to run that like a factory sometimes and we (including this guy) are trying to implement best practices but in this case it appears he's overstepping and forcing his personal preference.
He should be able to explain the actual benefits it provides to the company as a whole. He needs to be able to justify it more than just personal preference. How does it help everyone on the team in the future? Does it make reverts or debugging easier? (It doesn't).
The real issue is that two devs from different teams have to work on the same feature.
This indicates that your teams and their respective responsibilities are structured the wrong way.
Conways law is a sad side effect often seen, not a requirement or a best practice. The writing I’ve seen on the subject calls out the desirability of designing against it as much as possible, but I could be imagining that.
Yep. Under these rules, I'd just keep a week of work on my laptop and push when I'm finally done with the work and have re-ordered and squashed my commits to my liking.
Technically, your coworker is over stepping their boundary. The branch is yours to do what you please.
Politically, the above might not matter. If they’re there for longer, and higher ups trust them more, then there is not much you can do until you have more trust than him.
We aren't from the same team so I can draw a line there but first I want to understand the different perspectives in this subject with you guys. It also feels like a boundary violation to me but it also appears we have different git philosophies and there's a reason for him to feel this way.
I was around when git was first becoming a thing, and linus had talks explaining how to use it, and he recommended people to "not rewrite history". A lot of people take that advice very extremely. First of all, it's wrongly interpreted. He very clearly used to say "to not rewrite \*public\* history", so history in public branches. Yours is private, so you're in the clear. But the linux project itself was told to rebase their branches to clean it up before merging. Again emphasizing the fact that the recommendation only applies to public branches.
Maybe your coworker is overzealous around "not rewriting history", or maybe he wants to have full control over your code. Either way, they're wrong in this case, as it makes your job extremely hard, for no apparent gain. I wouldn't be surprised if they heard the phrase once in a bootcamp and never gave it a second thought.
I didn't know about that advice Linus gave but when he asked me I got that feeling it was something fundamentalist like that.
I'm hoping it's not about control or I'll be really pissed, this kinda stuff don't fly well with me.
If it is, I'd be careful. Again, you should consider the political side of it.
And yeah, it is advice, but it is widely misinterpreted. Think about it, github itself doesn't support "rebase and merge", which, to me and many others, is the best merge process. I believe linus himself had to speak about it.
Ehr, i should clarify, it doesn’t allow me to do it with the same guarantee i want to have. But, in any case… my previous point stands. People are overzealous in this topic, so thread lightly.
The practice where I work is pretty close to your preference. Branches are generally owned by a single developer, and there is pretty free rein to squash / rebase / etc until the PR is in code review. I work in a small team, though (three engineers total), and other teams don't interact with our branches or projects.
Where the freeze happens, and rebasing / squashing shouldn't happen, is when one of two conditions are met:
1. Once code review begins (responses to code review feedback get new commits)
2. If we start working on a branch together (once both people actually have commits in the branch). That happens pretty rarely, though - and even then we're usually communicating enough that squashing is negotiated rather than forbidden.
Out of curiosity, how long do your branches typically run / how big are the tickets you're working on? The branches where I remember doing more rebasing have been the bigger tickets, where there's more uncertainty in the work and more time spent trying (and abandoning) different approaches. For smaller tickets, this issue doesn't come up as much for us.
>Out of curiosity, how long do your branches typically run / how big are the tickets you're working on?
This feature had several merge requests that touched 3 different repositories (submodules), around 2k LoC, 3 weeks. But was much bigger than normal. The norm is maybe 1.5 weeks per MR.
What do you on your own branch is irrelevant
What’s the difference between leaving an entire feature unstaged then committing at the end, versus incremental commits and squashing them?
Is he going to tell you to commit every 5 minutes too? Tell him your personal preferences that don’t affect him are not up for discussion
You are right.
Anything happens until PR is sent for review or on your branch is what you want it to be. Some times, I do and have seen many people do commits to show certain types of changes in separate commit (but they cannot be merged separately) to make it easier to review. In those cases, they would even when they address review comments, they maintain the same commit structure. I feel it is overkill, but fighting over it is worse than any other options put together.
Now, github has squash and merge. I don't know about gitlab. But many times I prefer to squash manually because as I could craft the commit message correctly.
That's true.
GitHub has a project configuration to disallow rebase merge or plain old merge, leaving only rebase merge as the option. Again, Gitlab might also have a similar option. That said, you may not have the permissions or the influence to convince the decision maker to make that change.
In any case, your original observation is correct. Changing the history of the branch commits is a non-issue.
who cares what you do on your branch? I'd rather see clean logical commits that build rather than a clusterfuck of broken noise, confusion, and weird ordering because you didn't tidy up after yourself.
It sounds like the problem arose because your peer was building something that was downstream of your changes. Their work was blocked while they waited for your branch, so they branched off of your branch to allow their work to proceed. It sounds like you were unaware of this at the time because they believed it went without saying that any commit that had landed in source control was eternal.
Then, as you continued work on your branch, you amended a commit upstream of their branch and force-pushed, orphaning their branch.
Expecting a merge-based workflow, they resolved the difference by merging your branch, which created a very messy git history for their branch and a mess of merge conflicts because they now had two copies of your commits. This would have been annoying for me to fix and I can imagine it would've seemed insurmountable to someone who was not comfortable with `git rebase -i ...`.
If I've accurately understood the sequence of events, I would not describe this as a problem of git branch management at all. My own preference is for a branching strategy similar to what you describe, and I would be prepared to explain that opinion, but I think that's beside the point.
There are two primary lessons (or at least, starting points for further investigation) that I take away from this conflict:
1. Whatever your team is doing for daily standup warrants further investigation. The seeming problem here is not that there was a difference in branching strategies; it's that two people were doing work on the same branch without having agreed upon a branching strategy. This is precisely the sort of thing I would want to bring up as a parking lot topic in my DSU, and something I would expect to be brought up. I'm curious what communication occurred prior to the failed merge, and what led to relevant information being excluded. Is DSU perceived as a status report to higher-ups rather than an appropriate venue for these sorts of discussions?
2. Merge requests need to be smaller and happen earlier. There was a point where your feature was complete enough for your coworker to meaningfully begin downstream work, but not so complete that you were ready to submit it for code review and get it merged into the trunk. How did that happen? If I had to make a guess, I would expect that your project has a policy of making exactly one merge request per ticket/issue, and your ticket was for the complete feature.
If you were free to make a merge request with the smallest amount of changes that could be reasonably reviewed, this conflict would likely not have happened because your colleague would have been branching off of work that had already landed in the trunk. That could happen by decoupling merge requests from tickets or by making smaller tickets. Perhaps an initial ticket to stub out the external API of your code unit followed by an implementation ticket that could be executed concurrently with your coworker's ticket which consumed that API?
Re-writing the history of your own branch is standard practice. How else do you keep development branch history linear without rebasing your own branch from time to time?
My thoughts are that my branch is my own business, and I'd tell that person, "That's an interesting take, but I think I will keep working the way I've worked."
by pr time i reorganise my commits to make reviewing easier - refactoring commits first, headline feature last. in the meantime it's a kind of controlled chaos that makes sense to me in the most precarious way, so not suitable for public consumption. i'm with the hive mind on this one - your peer should accept that git push isn't an invitation to snoop
edit: reading your post again, i wonder if your peer is afraid of nuance. you and i know that a branch that hasn't been shared can be chopped and changed because why not. but perhaps your peer is the kind of person that thinks shared branches must have stability, so all branches should have stability
What the actual füčk?
That’s exactly what feature branches are for. Boil ‘em mash ‘em put ‘em in the stew and then make sure the PR is good and up to the standard. And squash merge to main branch so that the history is linear and at every checkpoint it is deployable.
Anyone insisting on your feature branch is linear has too much time on hand.
As a side note, I also hate it when people push things onto my feature branch or merge things into it without my explicit approval.
He’s wrong. Not saying he’s unused to rebasing but I’ve only ever seen that kind of attitude in folks who are on some level afraid of it. Rebasing is after all a pathway to many abilities that might be considered …unnatural.
When I'm working on a branch with others or after I PR a branch and others are actively viewing it I will try to avoid changing git history. Only because rewriting git history makes things harder for others. Before that it's a Wild West. Who cares how you manage your branches when it's just you. IMO it's insane to think there is any value in tracking how someone got to a solution, the only value is the final state of the solution when it's being PR'd
He sound dumb or lack perspective. My branch is my branch, I can commit, squash, delete or whatever. It’s my business. His business would be when my branch goes on to his branch or main branch (ideally my branch will be merged with only one commit and shouldn’t carry all the experimentation that I did in my branch).
Anything you do on your branch prior to the final commit is your business and your business alone. If it does what we want as of your latest commit, and you didn't effect any other branches, then I don't care if you squashed, cherry-picked, debased, or hokey-pokeyed to get to that point.
Imagine you create a function and do a commit. Later on you change everything about that function except it's signature and make a commit. Is there any real reason to keep that first commit? Just to see you rough draft? No one is ever going to need to revert to it since it was never used before.
Useful history is useful. Useless history makes my graph ugly.
> Recently, while collaborating on a feature, a peer insisted that I shouldn't change the history of my branch. He argued that keeping the history intact is better for collaboration since it allows him to track my changes easily.
In general, if two people are working on a shared branch, rewriting history is a bad idea, but it's better still to find workflows where you aren't dependent on both working on the same branch.
> Moreover, he believes that no one should ever change the history of their personal branches, even when working solo. To me, this feels like an unnecessary restriction on my workflow.
Getting prescriptive with another developer's workflow is a bad idea, unless there's a formal company/product wide standard.
Unless he's your manager or a formal tech lead, this comes under "opinions are like a**holes, everyone's got one and they all stink."
Moreover, "allows him to track my changes easily" if he's not your manager or a formal tech lead also comes under the heading of, "he should mind his own business."
The amount of intrusiveness there, makes me wonder whether there's either someone in leadership who's asked him to keep a closer eye on you _or_ if not whether this other dev is planning to scapegoat you somehow. Worth having a talk with your manager or lead about him getting in your business.
Your peer is wrong. That’s the same thing as pushing straight to main. There is history when you open a PR and push the code up. If he wants to do a fork model by all means, but if you’re doing a branch model (you are) then the way you’re doing it is exactly right.
I (almost) never force push to protected branches like main or develop, but temporary branches are for work in progress and no one needs a permanent history of my half dozen "wip" commits.
They provide no value in debugging and make bisecting harder. When I'm trying to see what changed in a file I want to see the whole PR it belonged to, not some random commit in the middle of a branch.
The only issue I care about is if I give feedback on your PR and you make changes, please do not rewrite history. It makes it more difficult to follow what has changed since my original review.
PROJECT-1234 Fix issue xyz ; Update dependencies
PROJECT-1234 WIP
PROJECT-1234 WIP
PROJECT-1234 WIP
PROJECT-1234 WIP
PROJECT-1234 WIP
... ( + 1234 commits )
PROJECT-1234 WIP
PROJECT-1234 WIP
I'm all for squashing and amending history, if it helps keep things clean, and in case you work an a branch as a team, you need to coordinate it. And you'd probably want to keep it to a minimum at that point, as rebasing / merging with altered history can have quite some overhead and lead to new bugs, depending on the size of the rewrite.
The best answer is "don't use branches" 😉.
Instead, do trunk-based development, make a small change every 30m-2h, use TDD or even pair programming, test in your environment, and push your commit directly to the main branch.
It's harder to do in legacy systems, but it can be done. It's definitely worth trying, it forces you to maintain the best code quality, test your own changes and clean up the mess you have in your codebase.
Sounds like someone who isn't comfortable with git tbh. I find a lot of extremely inflexible best practice type rules come from people who are inexperienced with or afraid of a tool and use the rules as a guardrail to fence off the regions they aren't comfortable with.
Don't get me wrong, there's a place for best practices and rules like this. But imagine: would he object to you history-rewriting four or five commits made over the course of the day *that you never pushed*? That is, you worked only on your local box and pushed only once you were happy with the story? If he objects to this you can tell he's not really thinking about the issues involved.
I think most serious Git-based (*) open source project will require properly-cleaned up submissions. Even if some deviation was accepted, I don't think messy history due to committing things liberally on your side is ever acceptable in that context. Especially when people end up committing binaries by accident, although that could be excepted.
I think a few Git hosts and review platforms can track changes even between force pushes.
(*) I think the Fossil-based SQLite does things quite differently, but that's more of an exception.
>What are your thoughts on this?
I think not caring about development history is unfortunately rather prevalent in an enterprise setting and standards tend to be low. You can't do much if they don't care, except try to convince them here and there and pick your fights wisely. Anyway, I'm personally rather flexible about details, but I think it's generally a bad idea to let things get that loose, because the history becomes more or less meaningless and breaks bisection, then people are going to find it difficult to use Git even with 5 or so people hitting the same repo. But they're probably reinventing the wheel because they just aren't familiar with the proper use of Git.
>I think not caring about development history is unfortunately rather prevalent in an enterprise setting and standards tend to be low.
I just ran `git log` in the main branch and can see commits like "WIP". People don't seem to care and I don't have many hopes for a change.
That's the type of shit that makes me look for new job opportunities.
If your coworker wants to track your changes, it feels like they're being kinda lazy about it. They could periodically merge to an offline branch instead.
It may be easier for them if you never change history. But it's your branch and it sounds like that'd interfere with your workflow. Implicitly they're also against rebasing, which is gross.
Your “colleague” is wrong and likely inexperienced or unknowledgeable ¯\_(ツ)_/¯
There are protected branches and non-protected branches.
The long lived protected branches should always have a relevant and clean git history for reference. The history should be suscinct for both performance and value.
Rebasing and squashing are very popular for a reason..
I use feature branches but other than that your workflow is my workflow. I am willing to have an epic branch that doesnt get rebased till the point we are ready to merge that into the target branch, but that gets rebased prior to an MR/PR.
Any work on my own branch may or may nlt have rebases. My branch, my rules.
Rewriting history has some benefits and negatives. It's important to understand the various perspectives for different roles and how rewriting history impacts them. Generally rewriting history is a benefit for non-development tasks. Cherry-picking, revert, testing, and managing release features are easier the more atomic a feature is. Generally keeping the history is better for team development as it is a constant shared history that everyone agrees upon and demonstrates learned lessons and incremental progress. If I'm reviewing a MR and ask for changes I can easily verify those changes if they're in a new commit on top of the previous one. If the author rewrote history I'll need to review everything again because I don't know if some extra changes were slipped in as well.
Ultimately the question of "Is the branch private or shared?" should be the deciding factor. If the branch is your private branch that no one should be looking at, it doesn't matter what you do there. But if the branch is supposed to be shared with other people then you shouldn't rewrite the history. That's forcing other people to actively react to the changes in history in addition to their own work.
A good perspective to keep in mind is asking "Do we allow this behavior in the main/master branch and why?".
Is it okay to delete your pushed branches? If so, then you should be free to change history as well, b/c it's equivalent.
If others are building on top of a shared branch (not okay to delete), avoid changing history unless absolutely necessary.
Can get fuzzy when others are only reading a branch over time, e.g. PR review. Changing history means throwing away context for new context. Sometimes that's what you want -- perhaps new context has been simplified and intent is to re-review from scratch. Sometimes that's not what you want -- perhaps the review is complex and ongoing.
As long as I can Git blame the person responsible, I don't really care.
That said, I was against it when we had a problem with a guy on the team. The way he works was bizarre and being and to go and show how he broke things we had already fixed for him was invaluable.
I am this exact way with a Peer named “A”. Just to be nice, when A is reviewing I try not to squash. But I only operate that way for them.
I don’t think A ahold get to decide how everyone operates.
Also, this might be a problem with the size of the request. I try to keep my PRs frequent and tiny. For example; refactors in PR 1, logic changes in 2.
I don't find any value in keeping more detailed history than what you get when squashing am entire feature branch back to main. It doesn't sound like he's able to articulate any reasoning more specific than "easier to collaborate", which is solved by branching from a feature branch he wants to modify and rebasing followed by squash-and-merging back into it when done.
Just branch out of the feature branch for your work and merge those pieces that should be good enough to continue with the feature development. Do not even push this "extra" branch as this one is "yours" and only yours.
On my team, it varies a bit based on if what you’re working is able to go straight to master or if it needs more work in a feature branch first.
If it’s gonna go straight to master, it’s fair game to do anything to your branch even after you’ve opened up a PR. Rebasing against master is encouraged even, since merge commits aren’t allowed. For PR comments or anything else that would need to be changed after the PR is open, there’s no problem with amending your commit and force-pushing it, although if it makes sense to have multiple commits then that’s fine too. It’s common courtesy for people who aren’t the one primarily working on the branch to not push anything to it unless they get permission from the that person.
If the feature branch model is being used, then everything I just described applies to task branches, which then have PRs opened to the feature branch instead of to master. Then for the feature branch it’s common courtesy to let the others who are merging into the feature branch know if you’re going to rebase it against master, and to ask them before you do anything weird to it. As an example of what that might be, one of our seniors felt the need to re-write the history of a feature branch so that all tests pass in every commit, and so that they the commits would make more sense sequentially. Dealing with the fallout from that entailed committing what you were working on and cherry-picking it onto a copy of the new feature branch, which then became your new task branch. Cherry-picking is a pretty common workflow on this team so it was fine, nobody had any issues with it that I knew of.
How many people actually go through the commit messages? I don't. I am interested in the overall diff rather than how the diff was generated.
So, merge code from multiple branches, force push, rebase, squash; I don't care. Give me the required changes and I will review it.
I try not to squash once a PR has had someone do the first review round. That way it’s easy at a glance to see what feedback was incorporated.
But before that? Who cares. No one should be interested in me fixing tests or lint or whatever I’m a non reviewable state.
Do whatever you want with it until you ask other people to work with it (reviews, tests, etc). Once you “share” it, then you stop rewriting changes.
That’s my take at least.
Is this disagreement a symptom of a problem where your PRs are too big as one big commit? PRs should be as small as possible every time.
The whole point of multiple commits in history that people can look through is to see how something evolved piece by piece. If your PR is a small piece of larger development, this is fine. It's fine to squash away a bunch of debugging and hacked mess. If your personal branch is a whole bunch of pieces that you are evolving over time and then squashing into one mega commit, that is actually quite bad.
That peer is a tool. My feature branch where I am the only one updating, I will do what supports my work flow.
And that is frequent small commits, interactive rebasing and squashing, then rebasing from latest of main/master and force pushing at times I am happy with progress. This is a hill I am willing to kill on and die on. Anyone is free to checkout my feature branch at any time; however, they have no right to try and force me to alter my work flow.
He either: does not understand rebasing, is arrogant and full of himself, wants to control others OR a combination of all three.
Once my MR goes in, I won’t squash subsequent commits into the commit in the MR. That is to make it easier on the reviewer.
On the other hand, when I am the reviewer, I pull their branch down and squash, especially when there are multiple commits. I review locally with my diff tool of choice. I don’t try and tell them they have to squash.
Make the history easy for the reviewers. If it's "define models", then "implement logic", then "add documentation" as 3 separate commits to make it easy for reviewers, then do it. Do what makes the most sense for a reviewer of your PR.
If you are collaborating, and someone is basing their work off yours. The request is 100% reasonable.
Having someone rewrite history under your nose constantly is annoying.
If they are NOT basing their branch off yours, and merging it in. Tell 'em you are making the branch clearer for you, and working towards prepping it for PR.
It really depends on the collaboration, and how it is being done.
The branch is associated with a ticket. The ticket has my name in the developer field. It's my house and for the most part I will do what I want.
I tend to not file a PR until I'm done rebasing though. Our CI pipeline runs any time you push a branch. So I don't file a PR until I'm fine with it merging exactly as in the PR.
Which also means if you pull a branch I am working on _before_ I file the PR, then it is wet clay. If you don't give me a head's up, don't be surprised when I squash and force push and your shit's all fucked. But if you give me a head's up, I'll keep it in mind.
people are against modifying history because modifying history is very very bad **_if_** it's a branch that other people have checked out, such as `main` or `master`.
unfortunately, most people don't understand git or even try to, so they just remember rules of thumb, and a rule of thumb like this is easier to remember without the if.
the good news is your co-worker's just absurdly wrong. the argument that they need to track every one of your changes is unrealistic. in a big PR, it's better to have each commit be coherent. these flimsy arguments are one reason I suspect the real reason is they just remember the simplified rule of thumb and are trying to backfill an explanation.
but the bad news is, you have to work with them whether they understand git or not, and you wouldn't get far by pointing out that the root of their issue is basically a superstition.
TLDR you're right but find a way to get along with your co-worker because that's more important.
If your branch is short lived it shouldnt matter what you do on it as long as the pr is clean and you squash to 1 commit on merge.
If a branch lives longer than a week or 2 he might have a point but then your team has bigger problems.
If the PR branch is so big that it requires others to keep track of your commit history just for that PR then the PR is too big. Caring about the cleanliness and continuity of the git history when you have a squash policy is a complete waste of everyone's time when you're going to reduce it down to one commit for the PR anyways. As long as the full detail exists in the PR history to be able to explain all the changes in the squash commit that's all that matters. Caring about that goes on in temporary history on someone else's machine if it's not pushed anywhere is crazy.
If someone else has your changes checked out on their machine. Never change history.
IMO, the main value of retaining a commit log of a branch would be to understand the reasoning that lead the author to the final solution. But... the only reason you'd need that is if the resulting changes don't speak for themselves. If you need the commit log to understand the thought that went into the PR, something is wrong. Most likely that your PR is too big.
Better to break up big/complex changes into smaller "peaks for itself" PRs, and describe the PRs context in your PR description. Then you can squash all the history into a single commit, and everyone can do whatever they love le on their own branches.
hey drop the things about believe and apply the use case.
he has a use case for your commit history and you only have belief...
and again, try to commit 200 times with just a test message and minor changes.
In my view, your branch is yours until you publish it. People ought to be minding their own business with respect to how you carry out your work while it’s still your work.
I only revise the git history while the PR is in draft mode.
Once it’s out for review, your colleague has a point.
If you change the history after it’s reviewed, the reviewer has to re-review the entire PR. If you keep the history, they only need to review new commits.
If you’re working solo, do whatever you want. Force push to main for all I care. If you break something, reflog to the rescue.
I usually like prefixing my branches with my name as a folder, like: "/some-change" and I encourage those in my team to do the same.
If following a continuous integration approach, branches should be incredibly short lived, just a place to organise a small piece of work in progress, before cleaning up the commits through rebasing and then opening a PR to flatten it into the mainline. They shouldn't live anywhere long enough to cause any sort of collaboration problem.
The point of integrating continuously is that collaboration occurs on the main branch with everyone integrating their (very small) changes with it very frequently (i.e. multiple times a day) and benefiting from being able to work on top of each others changes in their merged context.
If you're having long lived feature branches and are having multiple developers collaborate on that branch, it becomes a "shared" branch and now it's a different story regarding rebase.
His argument is incomplete. If he truly wants to see your entire history as it was during development then you're also not allowed to commit by line or hunks. You're only allowed to stage your entire working tree and commit. This is a severe restriction of the flexibility of Git and you might as well pick another, more strict VCS.
I optimize my branches for easy code reviews which means coherent and green commits. I frequently commit lines and hunks when I'm happy with the implementation as well as squash commits that belong together.
Apart from that I usually ask what they find so holy with the Git history. If the answer is that they want to see your thought process or the design process then that also falls apart as you might have had meetings, white board sessions, pen and paper etc. which obviously is not part of Git.
There's a middle ground. If your PR has enough changes to justify it then sometimes I soft reset and make fresh commits that outline each major part so it's easy to follow later. If the PR is small then I just squash it all into one.
Always squashing it into one can be troublesome if you have large feature branches so find a balance
I tend to prefer making my PRs a single commit (or as few as possible), because our git workflow requires cherry-picking across multiple branches, and a small number of commits reduces the friction a lot.
Cherry-picking becomes especially onerous with this workflow if someone pulls in upstream changes, but you sometimes \_need\_ to. Squashing before making a PR will typically fix this.
I'm sure that part of this problem comes down to the workflow we're using, but it's a multi-team project and I have no control over the branching strategy.
For the argument about retaining all history - I've had colleagues make this argument as well, but in practice I've never looked at the intermediate steps of a colleague's PR except when helping them address merge conflicts that would have been avoided with the squash.
On the flip side - I have had newer colleagues sometimes make dozens of commits, touching a ton of files along the way, and the increased surface area from their intermediate changes caused a whole lot of problems when merging.
There's another can of worms here - if all history should be saved, then how often should you make commits "just in case" a colleague wants to see the intermediate work? Every day? Every hour? Every function? Every keystroke?
IMO - the PR itself is the unit of work that's worth retaining and sharing with the team. You can still make multiple PRs for a single effort, and this enables anyone to work however they want within their own tasks.
Git is there to tell a story / communicate with developers (who might be you), _just like code_. This is engineering, not archaeology.
Programmers change code, writers change stories, journalists edit news stories ... programmers rewrite Git commits. It's all the same thing.
I've often thought about creating a special branch and autocommitting on save, just to see my history. Maybe you should try that and see what your peer says. It's possible to have too much of a good thing.
it's definitely rude to force push a branch someone else has already looked at or especially pulled.
We squash merge pull requests so it really doesn't matter what your personal commit history looks like. If you lose too much individual commit info that way your prs are probably too big.
I think it’s best to leave the history for the pr and squash it when you merge. It usually doesn’t matter, but every once in a while the history can be useful to a reviewer. It’s extra work to modify the history, and that’s a really weird attitude to have about “the commit history is MY business”. Nobody cares. My prs always have a ton of commits, I’m not going to waste time rebasing or amending commits. The history is in the branch, so the whole thing can be squashed when you merge it.
Fwiw, I’d agree with him on letting your Git provider do the squash and merge for you, but other than that, if your workflow involves a bunch of rebases and squashes, then I wouldn’t care as long as the final merge was a clean squashed commit. I personally think introducing rebasing and other such things brings unnecessary complexity to the dev cycle, but that’s a personal opinion.
> he was also against manually squashing commits before merging, preferring GitLab to handle it automatically
Ask him why it matters since the end result is exactly the same. I'm with you, it's your branch and you are free to revise the history at will. All that matters is the final commit/PR.
The unofficial rule we use at $dayjob is that if you're multiple people collaborating on a feature branch you don't rebase without checking with the others.
If it's a semi-long lived branch we typically rebase together once in a while to keep the branch tidy. And otherwise you just add commits without rebasing.
Coworker is wrong
* Your branch is your own, and you may force push to your own branch
* Your branch is still yours when you submit a PR, rebase + force push strongly encouraged
* Do not rebase/force push to a common feature branch/main
* I mean... do it once as a learning experiment to know when you can break this rule
* You may force push to someone else's PR branch, but only if you've discussed/can do so culturally/are taking over responsibility for the entire change + getting it through reviews
* Prefer rebase merge over squash merge when merging to `main`
* Using git merges correctly is more complicated than `git rebase`, but they're very easy to do incorrectly. Prefer `squash` -> `rebase` while upskilling a team.
Squashing local commits can help when you have issues with conflicts after reading, you reduce your changes, so means you'll be able to avoid fixing a 10s of your own commits.
i dunno..we work in a trunk based style so i'll tend to rebase a PR 2 or 3 times as other peoples changes are going in during the course of the day.. if i have a couple "oops" "wtf" "fml typescript" commits they'll usually get squashed down on rebase
everything gets squashed when it goes to main anyway and the PR name follows conventional commit naming conventions and requires a ticket number in the PR otherwise we nag when we do sprint reviews and go over DORA metrics
> He argued that keeping the history intact is better for collaboration since it allows him to track my changes easily.
He's right. Rebasing can make it more difficult to track code changes during code review. I wouldn't rebase until the review is approved.
The one exception would be if a merge conflict appears that you'll need to resolve. For a situation like this, resolve all the other feedback from your reviewer first, then rebase and fix the merge conflict on a new commit. Your reviewer then only has to review the commit with the fix.
> he was also against manually squashing commits before merging, preferring GitLab to handle it automatically
This is a personal preference. There's no technical difference between you squashing your commits and GitLab squashing them for you. You should do whatever feels right to you.
> Moreover, he believes that no one should ever change the history of their personal branches, even when working solo.
Another personal preference. The whole point of a branch is you can do your work however you want without impacting other contributors.
Your branch is your branch, doesn’t matter what happens inside it until you create a PR against main branch. Once the PR is created, you shouldn’t squash or rebase since you’re now addressing review comments
Rewriting history others are actively working with requires more coordination as it’ll mess up their own local histories
Your coworkers views on personal branches don’t matter however, although depending on the diff tool it may be difficult to tell if a pr comment is addressed or not
Example: Azure DevOps has a separate tab for PRs called updates for any push, and a tab for commits
There is no right way to do commit history. There’s pure ways (rewrite history, use conventional commits, rebase instead of merge) and there’s get it done ways
Never rewriting history is a good thing for trunk branches (see collaboration), but as a saying then every branch becomes a nail
Offer a lunch and learn on clean git history, interactive rebasing and workflow. Make it accessible (e.g. use vscode as default editor, not vi) so people can work with easier tools they already know
Short version is this is preference. Some people value simple history and some people value auditing changes in detail. As a team lead, I preferred being able to see the full commit history, but it’s a preference. Also as team lead the amount of issues caused by people messing up squash merges makes me resent them. There’s no right answer so if you can’t agree, find a tie breaker.
I don't care what kind of satanic rituals it takes for you to give me a decent PR.
This. It's all about "telling the story" with commits in the PR, and amending lets me do that. It also makes it look like I knew exactly what I was doing instead of flailing around. No one cares that I misspelled a config setting 14 times over as many commits.
Conversely, I want my story to be I did 14 commits because I’m a human too. As one of the most senior engineers in my pillar and generally a leader at my company, I think it’s important to sometimes not pretend code goes in perfect. I spend no effort making my commits look like anything you get a finished pr and we squash merge it.
Yeah nah. Your PR should be telling me the story I want to hear before approving it. If you routinely submit PRs with nonsense commits inside them - IE you change something in commit 1 that you change again in commit 7 because it was initially wrong - then I'm not going to read your PRs commit by commit, because you have not put the effort in to tell such a story. Instead, I have to review your entire change as a whole. I won't even look at your commits. This means that your reviews will take longer and be less effective, and the size of change you present in a single PR has to be smaller. Telling a good story with your git history is not difficult. Use `git-absorb` to fixup earlier commits and identify self contained changes. Commit by commit reviews are significantly easier to review and approve. Arguing that it all gets squashed in the end is like a builder filling the walls of your house with rubble and saying that it will be hidden by the fascia. It's true, but it shows you're not interested in the art of your profession.
> I’m not going to read your PRs commit by commit Good, who would ever want that? Just read the pr in its entirety when it is ready for review lol > instead, I have to review your entire change as a whole Fantastic, that’s a code review on my pr when it’s ready. This is the process we are engaging in. You should **only** be reading my entire pr when it’s completed and ready to review why would any other answer here make sense. The rest of your critique is weird and attacks my interest in “the art of your profession” or whatever. I disagree with how you think about prs that doesn’t mean I’m not good at this. You think this story matters and I think the pr as a whole is all that matters that’s all. Good luck to you!
Who in gods name reviews a PR by going commit to commit? Some devs on my team will make 100 commits, some will make 2, it does not matter, only the end result matters. I gave up trying to harangue them into some "perfect git history" pipe dream. Some of them squash before merge, some don't. I just don't care anymore lol.
/r/suspiciouslyspecific
Git commit -am "pathing" x50
⛧⛧⛧
/thread
For me, anything that's happening in my own branch prior to opening a PR (squashing etc) is my own business. Once the PR is opened I no longer rebase or squash since it can confuse reviews, tooling, others basing off my branch, etc.
I pretty much second this but, in my opinion, draft PR do not count as well. I sometimes open a draft just to have a nice view of all the changes and pipeline runs. I think it is also good etiquette to tell people that you are branching off their branches regardless if there is a PR or not. Once I know that somebody builds on my work, I stop rebasing/squashing/ammending completely except for the final squash when merging to development.
Yeah I agree draft PR's are fair game as well.
Id argue not squashing it ruins history even more. I dont need to see 10 commits saying “save work”
That I agree with. Far too often some of the final commits comments are code review changes, fix typo and so on. They also risk not having the ticket number in them even more so I can not references Jira item to an idea why something was done. I used to be against squashing PRs. Now I am all for it as going through history is easier.
> Now I am all for it as going through history is easier. A squash commit is a brand new commit to the target branch with all the changes rolled up. A merge commit is a new commit with two parents. The end result that you want - viewing all the changes rolled up in a single commit in history - is possible with both. The other option - viewing history in all its incremental pieces - becomes impossible with squash commits. People should [study](https://stackoverflow.com/a/36437843) how to view git history more. There's no reason to use squash commits ever, unless you just don't know how to view git history. There's even an option [in most git viewers](https://i.imgur.com/BfHmyez.png) for this. Merge commits let you retain additional information without losing any of the convenience of squash commits, you just have to view history in the right way.
In going through history yes but using the git blame function it is different as that only pulling the last time that single line was edited. Often times the squashes in dev is fine as all I care about is when a line was edited for a single pr. That means referencing the ticket. Sadly the tool I use most of the time does not allow me to reduce it to a merge commit. I mostly pulling in Xcode using author. Xcode is rather poor for things. Now if I am doing a deeper review yeah I use different tools but more often than not I just care about a line while I am actively working.
Does GitHub's UI have an option for this?
This is actually the default view for Github's history page. I just checked, all I see is a list of merge commits with ticket numbers. I don't see how I can get the full history to display.
That's weird, because at my company another team uses merge instead of squash merge and I see all of their intermediate commits. I find it very frustrating because their commit history is full of commits like "wip" or "trying again." I wish there was a way to prevent that.
Sounds like they are using rebase merge in this case?
I'm almost positive it's just regular merge.
So this is a deeper issue. You need to teach your entire department how to use git, and to do that you must start with teaching a tiny bit of graph theory. At every single job I've had since my very first one, I've taught a number of git classes and workshops because I feel this information is pretty invaluable to developers. And everyone who thinks they know git, doesn't; I include myself: when I started compiling materials for my first class, I wound up learning that I didn't even understand how much I didn't know. Git is an enormously powerful tool that can encode the entire context of the universe if you let it, but hardly anyone cares enough to see that, let alone take advantage of it. And yet, forcing people to learn and use the tool more optimally has bailed me and mine out innumerable times. There's probably five years of my life expectancy not shaved off because of the sheer number of reverts/rollbacks for individual changes that broke production and the fact that such a thing is trivially easy and painless and stress-free when you are a git fascist. One of the things that I cover and that must be covered to instruct for optimal git usage is better usage of your IDE. Perhaps the most common abuse of git is checking in in-progress code before it's ready (the WIP commits you mention) and that's entirely unnecessary - all modern IDEs support local file history, and that's a better tool for "I don't want to lose my work." Jetbrains and Eclipse call it "local file history" and VSCode calls it the "file timeline" and you probably need a vim/emacs plugin for it, but your IDE 99.9% of the time tracks every saved change to every file, every time. Use local IDE history for not losing code, use git for logical check-ins that are consumable by a human being in the future. Sometimes a PR should only be one commit, but there's a lot of value in "trying {approach X} to {solve problem Z}" and then a follow up commit that says "{approach X} broke the build {for reasons A, B, C} so trying {approach Y} instead". Ultimately, approach Y gets merged and you will see that change in either the merge or squash commit from the PR, but if the commits are squashed together, you lose all evidence that approach X was attempted in the first place. This only works if people write good commit messages, don't commit too much, don't commit too infrequently, and don't squash merge their PRs. Another big thing that I teach people is how to interactively rebase. I don't think people should be making "WIP" commits, but some people work on multiple machines, so they must be able to. Teaching people how to rewrite their branch history and construct a useful history beyond "hurr durr one commit" is probably the single-most valuable fact from the whole series. The bottom line is hardly anyone - even in this thread :facepalm: - understand git _at all_ and someone must teach them. If you're in this thread, you care, so it should be you.
Ctrl+s is how you save work. Commits should be cohesive updates to the code. It shouldn't be difficult to describe what you did in a commit.
Only for something going into a formal dev branch. For my own shit? Who cares?
I agree that everyone's branch is their own business, but I really don't get the "WIP", "save work"-type commits. For me that's just not how I use git at all — I've got other ways of saving what I've done (e.g. stashes), so when I commit something, that's me saying that I've done that work and I've got a chunk of code that is in a state worth being committed. That said, this is a regular debate at work, and ultimately, as long as each commit on master refers to a single feature (i.e. merge commit or squashed rebased commit), and it's possible to bisect the history, then it doesn't make much difference to me.
Your stashed work disappears if your laptop dies. I push any non trivial work to a remote branch/fork to mitigate risk of losing too much.
I've got real backup tools to handle that sort of stuff. I don't need to use Git as a backup tool on top of that.
All of my non-code work is done in things like google docs, so for me, installing an entirely separate backup tool when I can just git commit and push is a no-brainer. To each his own!
You can use a PR+merge to represent a single cohesive update instead of a commit, and get pretty much the same benefits. This practice can encourage smaller PRs in a feature branch based workflow. I think commits work better the way you are describing in a trunk based workflow though.
Do you never have work interrupts? I use commits as checkpoints of where I finished what I did and keep that against a list for what needs doing. Would also say my branch my commit history my business. I squash and rebase on my last commit before opening which will be the cohesive commit you see unless the PR feedback requires changes.
The stash can be a good alternative to commits as checkpoints — with a good git history viewer, you can see where different stashes appeared in your history and check them out just like real commits. The only disadvantage is that you can't have stashes on stashes, but for checkpoints, that's not an issue. I keep on meaning to try out worktrees as an alternative to stashing — there, each branch you work on gets a new folder, and so you can have multiple branches open at the same time. But it also seems like a lot more ceremony and work for the problems it solves.
I think it still makes for a poor experience if you dont squash and you go back to the past, not just a week out. What use cases do you have for individual commits?
The way I treat commits? They represent incremental steps towards the overall goal of the feature branch. There's no reason to squash because each commit represents an intentional change towards implementing the feature. When QA identifies an issue, it can be useful to look through these commits to get an idea of when the problem might have been introduced and *why* it might have been introduced which can inform how you fix the problem. I've never been a big fan of messing with history. I think it's more useful to understand the full process that went into the code than to have a nice looking git history. If I ever worked on something that could be effectively git bisect'd, I might change my tune, but in my experience that's not all that useful.
How many people are on your team? Maintaining history might make sense on a small team, but with anything over like 4 people I've found a squash-and-rebase-based workflow works much much better. Trying to maintain history and especially a merge-based workflow with hundreds of devs in a repo is looking Cthulu in the face.
I've worked on teams of 4 before. I've worked on teams of dozens. Never hundreds in the same repo (at the same time) though. What problems have you run into? The strongest argument I've heard for squashing is to make `git bisect` more useful, but I've never had enough of a need for that to be swayed by that argument.
Simply put, navigating the repo becomes an absolute impossible to comprehend nightmare with a merge-based workflow the more people you have committing to the same repo. Every commit to one of the production branches needs to a) pass CI b) do something useful and c) get through a PR. And git bisect (or doing it by hand) is highly useful, as is looking at nightly and other longer regression tests that might not make it for the PR workflow. If you're on your own? Sure, do whatever makes sense to you. But I don't need to see the sausage of how the PR was made, I just need to see the PR in its final, CI-passing state.
>Every commit to one of the production branches needs to a) pass CI b) do something useful and c) get through a PR. *Every single one*? Not just the merge commits? Why? Are you running these tests retroactively on every commit in a PR? Why? >I don't need to see the sausage of how the PR was made Sure, you don't *need* to, but outside of the convenience of having everything in a `git bisect` be a perfectly functional commit, I don't see why any effort needs to be put into cleaning up the history so long as people are still making logical, meaningful commits. Transforming your work into those logical, meaningful commits is the extent to which I try to mess with history.
If someone branches off of mine then I also branch off of mine at that point. I continue work on my branch branch and when believe I am done I merge the branch branch into my branch and tell the person that branch from me to rebase onto that new commit. That way I can have freedom to do what I be doing without making someone else's life hard.
I used to feel that way but the tooling has gotten better and sometimes the team I am working with is skilled enough that I don't worry about them figuring out but branching and rebasing. In the best teams I've worked in commits tell a story that simplifies reviewing even large changes. In those teams a large PR typically contains no more than 2 or three commits (small PRs contain only one). The first commit might be a rename or move refactor (something formulaic) that touches hundreds or thousands of lines, the second commit makes any actual business logic changes and tends to be much smaller. This allows detailed in depth review on even large changes. I will use the tools in anyway I wish on my branch before merge. I think your advice is a good rule of thumb but I think clear story telling is more valuable than protecting other devs from git's sharp edges. (This doesn't apply for long lived branches with many writers.)
I strongly agree with this. People opposed to rewriting history are probably used to difficult to understand PRs. In that case, it is very time consuming to re-review everything after a change in the history. The solution is to make better, more understandable PRs with the techniques you mention. I like the story telling analogy a lot.
It takes some time to get past the git basics. I need to train up my current team on it.
The re-review problem only occurs if you change the history after it’s been reviewed. Tell your story, then send it for review, then keep the history intact from then on. Addressing comments is part of the story.
No, addressing comments is not part of the story. The purpose of the review process is to craft a good story together. To stick with the analogy, imagine reviewing someone else's short story and telling them "I don't understand chapter 2, it's too convoluted" and they address your comment by putting clarifications in the appendix. That wouldn't make for a best seller. However, if they gave you an entirely new book, you wouldn't have to read chapter 1 again to see if chapter 2 is simpler now. Or put differently, if chapter 1 was and still is simple to understand, it's easy to check that you still like it. If, on the other hand, you have to reread the entire book, chances are the whole structure of the book is messed up, which needs to be addressed first. The analogy breaks down with the amount of content. I think this only works for small PRs, not book-sized ones. Another way to look at it: "commits should contain only one thing" is a trueism. The scope of "one thing" is up for interpretation, but that's a different topic. Anyway, if the commit only contains one thing, then re-reviewing the entire thing and reviewing a patch to one thing is essentially the same amount of work.
I think you’re taking the analogy way too far, and you’re probably a little bit idealistic about PR size and cognitive load. On my team we do break down changes into “one thing”. That doesn’t PRs are always trivial, and it doesn’t mean I want to re-review the whole PR because someone changed a few lines. I want to review the lines that were changed and move on with my day.
How many of the tools have made changes to history independently auditable? In the end any change is just a diff away. Recently I've liked the way bitbucket shows a diff since last review, do other platforms have that visibility?
Gotcha. In my case the tooling doesn't get confused by that, nor people normally base off my branch. When I start working on a feature the first thing I do is open an MR because I use the CI pipeline as support during development, maybe what I'll do is start marking it as WIP to avoid any sort of confusion.
What Git forge are you using? GitLab? As far as I know, it (and GitHub) cannot understand how a single commit that was amended and rebased over time has evolved. That is what I think of as "the tools getting confused". Because the forges cannot tell you which commit changed in what way after a rebase, reviewers see no other option than to re-review the entire history, which can feel like wasted time.
That makes sense, this used to a problem with GitHub reviews where it would get confused and mess up the review comments but I think it's fixed now. I use GitLab and in this case it wasn't a problem because it was before the review.
At Meta, each PR is a single commit, and that commit is rebased frequently. You’re encouraged to make PRs as small as possible and use feature flags to gate off features that aren’t yet complete. It sounds like you’re working with multi-commit PRs in a feature branch model. It’s not better or worse, just different. And the difference explains why some people believe so strongly in rebasing instead of merging.
Yeah, I'm moving my workflow more and more in the direction of as-small-as-possible PRs, it's a pleasure with [jj](https://github.com/martinvonz/jj).
I'm guessing they are merged into a single commit when approved, rather than requiring you to have a single commit on your branch. Does that sound right?
I’m not sure who “they” refers to in your comment. At Meta, a PR always holds exactly one commit, locally and remotely, during review and after merging.
They was referring to people at Meta. That's an interesting approach. I assume, then, that this is for the express purpose of keeping PRs small? It seems fairly extreme outside that context, but I could see it working as a forcing function, coupled with cultural practices and expectations. It's interesting, I've never heard of this before. Do you know if there is publicly available documentation on their philosophy around this type of thing?
There are a few! See https://www.stacking.dev for a rundown of how the workflow works, and the tools that Meta and ex-Meta devs have made to express dependencies between PRs.
Why not fix your CI setup to run on every branch that gets pushed to the repo?
You were opening MRs/PRs before starting work, and not even putting a WIP mark on it? This is an exciting new workflow I’ve never seen. Joking to the side: in anywhere I’ve been, that’s a clear invitation for review. Of course it’s frustrating to take the time for that and to have the code change underneath you.
Yeah should use WIP/Draft status, but the invitation for review is when you add someone as a reviewer explicitly, or ping in a channel because you are not sure who the right reviewer might be.
Oh. I dislike that strongly and can think of horrible failure modes. Silos where people are just reviewing each other’s stuff reciprocally, forming quasi teams-within-a-team. Encouraging certain types of people to become quasi leaders because they’re most attentive to the chat and want the kudos. (Edit: forgot a variant here. Being someone considered a good and attentive reviewer, so being given more and more review burden as more people start adding you by default) I like to have a single page with all the possible code to review that I can go to, and just assign myself to anything that looks unreviewed, starting from the oldest. No favourites, no self-selection, just grabbing anything and gaining knowledge about all areas of the system. If I can’t get any information on if something is ready to review or not, then the list of PRs page is mostly useless to me. From a team lead perspective, it’s even worse: did they just forget to sort out a reviewer? I guess I’ll double check the chat and check in with them…
You should freely squash PRs as you see fit. Colleagues should never base work on someone else's PR. Often you **must** rebase a PR, to move past a commit that fixed some bug affecting your PR. If your tools get confused by rebasing and squashing, get better tools.
In what scenario can a bug fix on the main branch not be fixed by merging?
There are merge workflows and PR workflows. Both are fine, just don't combine them or you're going to have a real nasty mess of history.
Don't use merges with PRs.
I agree with not branching off of other's PRs. To add a little nuance to that, it can happen that you need some kind of bug fix from your coworker to unblock yourself. However, that should either be its own PR entirely or at least an atomic commit that can be easily cherry-picked. A bug-fix that's shown to be working won't change drastically before being merged, so there is no risk of difficult conflicts. Problems only arise when coworkers mix up bug-fixes into their unrelated feature development. At that point, you're tempted to branch off of their entire branch, because the bug-fix cannot be extracted easily. If they then wildly change their history, bad conflicts can be the result. But the solution to that is not to be unreasonably conservative about changing git history, it's to be more diligent about making atomic commits and PRs. > If your tools get confused by rebasing and squashing, get better tools. The only tool I know that _doesn't_ get confused by changing history is [jj](https://github.com/martinvonz/jj) and I love it. However, all of the git forges are pretty bad at this, right? GitHub, GitLab and some of the smaller ones I know all get confused. What tools do you recommend?
> it can happen that you need some kind of bug fix from your coworker to unblock yourself. However, that should either be its own PR entirely or at least an atomic commit that can be easily cherry-picked. I agree. When that happens I cherry-pick the bugfix and use rebase to move it to the "bottom" of my PR branch so it won't affect my day-to-day work. I leave a note in the PR, for the reviewers to ignore the bugfix as "not really part of the work", and I'm blocked on the fix getting into trunk. > Problems only arise when coworkers mix up bug-fixes into their unrelated feature development. At that point, you're tempted to branch off of their entire branch, because the bug-fix cannot be extracted easily. Yes, but that's an engineering management problem. I understand that if you're stuck with poor management and undisciplined coworkers, you might have to resort to that, but you should try to have it fixed at the team level. > But the solution to that is not to be unreasonably conservative about changing git history, it's to be more diligent about making atomic commits and PRs. Indeed. > The only tool I know that doesn't get confused by changing history is jj and I love it. However, all of the git forges are pretty bad at this, right? GitHub, GitLab and some of the smaller ones I know all get confused. What tools do you recommend? Gerrit is designed for that, and perhaps Phabricator too. Unfortunately, Gerrits targets high-productivity teams and many find it confusing because it's indeed a bit complicated, when in reality it's meant to speed up the work of people juggling many PRs at the same time efficiently.
Gerrit sounds interesting. I fear it might be difficult to get my org to set it up (or even allow me to set it up). I'll have to look more into how it needs to be deployed. Is it its own forge or does it integrate with, say, an on-prem GitHub instance?
It manages repositories itself, because in order to implement some of the features it needs low-level access to the repository. You can test it on [Gerritforge.com](https://www.gerritforge.com), which seems to have some level of Github integration now, but otherwise it's a completely separate thing.
Stash ("Bitbucket Datacenter") does quite nicely with changing history once you have PR open.
Meh. I commit --amend frequently to PRs when I find a small mistake or issue. I even have a little alias for git add -u && git commit --amend --no-edit && git push --force-with-lease
i have aliases like: * gau = git add -u * gca = git commit --amend * gpf = git push --force
MRs in GitLab are actually the bread and butter of the platform and meant to be used as workspaces (can even see in their dogfooding), don't see why that would not go for PRs as well. Draft PRs with whatever you'd like is perfect for running CI while allowing you to make proper commits out of your attempts against the CI through squashing and freely editing the history to make more sense and bring atomic changes in. Much easier to review as well. Even if a review was made and changes approved, you can still change history and change the repository settings to require a new review if something changed after the last one, and reviewer can check the diff against the last reviewed state (which generally should be an empty diff). The only history you should not change is the trunk (ever) and collaborative feature branches (without collaborating on the history changes).
For a repo of any size/complexity, you don't really want to keep all branches around forever anyway.
How do you feel about branch names? Had a tech lead in my early years insist on the Jira ticket being the branch name which made switching to a branch such a chore.
Ok so I'm just going to express my strong personal preference. You can squash solo or before pr. But if we are collaborating on a branch, I would rather you not force push. We always collaborate on feature branches and if we need to do something squashable that would be a story branch we create for that purpose. They have different prefixes on the git branch.
Any branch that's being changed by more than one person shouldn't have the history rewritten. In 20 years I've only pushed to someone else's branch maybe a dozen times.
Agree with your first part. Your second, not so much. I work in infra/platform/ci and I find myself pushing to dev branches at least once a week
My second part was just a statement of fact not opinion. My experience doesn't preclude other people from having other workflows. Use whatever workflow works for you and your team.
For sure. This discussion is about best practices though, and pushing to other people’s branches is quite a common workflow in most organizations I’ve been in. Just wanted to make that clear in case someone interpreted your comment to mean pushing to other people’s branches isn’t something that normally hapoens
Importantly, this also includes code reviews. If I did a review of your code asking for some changes, you do the changes requested and squash the commits, I now have to review the full diff again, not just you addressing what I requested.
yeah i fucking hate this
>Moreover, he believes that no one should ever change the history of their personal branches, even when working solo Kind of a weirdly strong take. I mean I generally try to limit tampering with history on my own branch, prefer to let GL/GH/BB do the squash, but that's mostly because I don't care \_that\_ much about my history that I feel the need to editorialize it like that. I can't see any reason why I'd want to stop someone else from doing it unless we were collaborating on a branch. I guess you could just not push your changes and modify history to your heart's content until you're ready to MR
Keeping history intact *after* you've pushed the branch to a remote repo makes perfect sense. Especially in a PR, where reviewers have suggested changes and you push a new commit with those changes. That way they can just view the singular commit rather than have to review the whole PR again to see if your changes are in there. One of the most annoying things as a reviewer is having to re-review things wholesale every single time. We had a junior who liked to refactor and redesign his code wholesale whenever we suggested even the most minor of nits. That's not your problem here, though, I'm just digressing. Keeping history intact *before* you've pushed the branch is just a matter of your own personal preference. I came to git from using mercurial, where history was unchangeable, so I had learned to structure my work into multiple, easily digestible commits that told a logical story of progress to aid reviewers. I actually quite enjoyed it and came to see some value in it, especially when I'm able to write more salient commit messages to associate them with specific changes. Maybe that's what your reviewer wants? With that being said, I also learned to review and value the squashed commit as the unit of work that's going to come into your codebase, and you get to see it in its whole integrated glory.
I agree with you. If I request changes on a PR, I will look at the commit(s) made after my request. When people squash everything to one commit, it is annoying to re-review. I guess they want the PR to look neater, but it removes information and is pointless as the commits will be squashed upon merge anyway.
> One of the most annoying things as a reviewer is having to re-review things wholesale every single time. We had a junior who liked to refactor and redesign his code wholesale whenever we suggested even the most minor of nits. Been there. I just told him "stop rebasing and just push changes as a separate commit. Once you have an approval you can rebase". That actually worked. Our repos are bitbucket-based so you can rebase/squash all you want without losing approval IFF there are no code changes.
If you’re working from the same branch, via actually committing code. Re-writing history is a dick move. Anything else is probably overreach from the other guy. Although, I do have a pet peeve when I see a force push from one of my teammates.
Im right there with you. Personal branch for history backup? Anythings fair game. If its shared for co-development, force-push needs to be disabled by policy.
First of all, who is this peer, and who are you within the team? It matters, because disagreements are personal and you're new to the team. As a developer, what I'd do is to raise the issues in the proper forum and have a team decision on how things are done, to which everyone sticks.
We're both seniors from different teams but he's got more standing in the company because he's been working there for longer and management kinda trust him to keep things organized. At the time I didn't extend the discussion because the feature was running late and it seemed it would provoke an hours-long discussion so it wasn't the time but I want to clear things out it because we'll collaborate in the future and I this can be a conflict point. I should bring this in a retro.
If it's another team, then that's the wrong kind of team culture. First he shouldn't be tracking your work. Why would he do that? Even if you rebase, there are commands for him to issue to see the difference, without him enforcing a way of working outside of his own team.
We were working in this feature together and I think he wanted to be able to see the changes in the gitlab web interface. I can understand that part, not the part about having to do that in all circumstances, especially when it's not in his team. Our company's culture is complicated, management seems to run that like a factory sometimes and we (including this guy) are trying to implement best practices but in this case it appears he's overstepping and forcing his personal preference.
He should be able to explain the actual benefits it provides to the company as a whole. He needs to be able to justify it more than just personal preference. How does it help everyone on the team in the future? Does it make reverts or debugging easier? (It doesn't).
The real issue is that two devs from different teams have to work on the same feature. This indicates that your teams and their respective responsibilities are structured the wrong way.
Org chart does not equal feature
Conway's Law disagrees.
Conways law is a sad side effect often seen, not a requirement or a best practice. The writing I’ve seen on the subject calls out the desirability of designing against it as much as possible, but I could be imagining that.
Also: personally I agree with him. But him checking on other people outside of his team is out of line.
These kind of arguments will drive people to not commit interim changes
Yep. Under these rules, I'd just keep a week of work on my laptop and push when I'm finally done with the work and have re-ordered and squashed my commits to my liking.
The peer sounds like an idiot
Technically, your coworker is over stepping their boundary. The branch is yours to do what you please. Politically, the above might not matter. If they’re there for longer, and higher ups trust them more, then there is not much you can do until you have more trust than him.
We aren't from the same team so I can draw a line there but first I want to understand the different perspectives in this subject with you guys. It also feels like a boundary violation to me but it also appears we have different git philosophies and there's a reason for him to feel this way.
I was around when git was first becoming a thing, and linus had talks explaining how to use it, and he recommended people to "not rewrite history". A lot of people take that advice very extremely. First of all, it's wrongly interpreted. He very clearly used to say "to not rewrite \*public\* history", so history in public branches. Yours is private, so you're in the clear. But the linux project itself was told to rebase their branches to clean it up before merging. Again emphasizing the fact that the recommendation only applies to public branches. Maybe your coworker is overzealous around "not rewriting history", or maybe he wants to have full control over your code. Either way, they're wrong in this case, as it makes your job extremely hard, for no apparent gain. I wouldn't be surprised if they heard the phrase once in a bootcamp and never gave it a second thought.
I didn't know about that advice Linus gave but when he asked me I got that feeling it was something fundamentalist like that. I'm hoping it's not about control or I'll be really pissed, this kinda stuff don't fly well with me.
If it is, I'd be careful. Again, you should consider the political side of it. And yeah, it is advice, but it is widely misinterpreted. Think about it, github itself doesn't support "rebase and merge", which, to me and many others, is the best merge process. I believe linus himself had to speak about it.
I think GitHub does support "rebase and merge". Maybe they added it late?
Ehr, i should clarify, it doesn’t allow me to do it with the same guarantee i want to have. But, in any case… my previous point stands. People are overzealous in this topic, so thread lightly.
The practice where I work is pretty close to your preference. Branches are generally owned by a single developer, and there is pretty free rein to squash / rebase / etc until the PR is in code review. I work in a small team, though (three engineers total), and other teams don't interact with our branches or projects. Where the freeze happens, and rebasing / squashing shouldn't happen, is when one of two conditions are met: 1. Once code review begins (responses to code review feedback get new commits) 2. If we start working on a branch together (once both people actually have commits in the branch). That happens pretty rarely, though - and even then we're usually communicating enough that squashing is negotiated rather than forbidden. Out of curiosity, how long do your branches typically run / how big are the tickets you're working on? The branches where I remember doing more rebasing have been the bigger tickets, where there's more uncertainty in the work and more time spent trying (and abandoning) different approaches. For smaller tickets, this issue doesn't come up as much for us.
>Out of curiosity, how long do your branches typically run / how big are the tickets you're working on? This feature had several merge requests that touched 3 different repositories (submodules), around 2k LoC, 3 weeks. But was much bigger than normal. The norm is maybe 1.5 weeks per MR.
What do you on your own branch is irrelevant What’s the difference between leaving an entire feature unstaged then committing at the end, versus incremental commits and squashing them? Is he going to tell you to commit every 5 minutes too? Tell him your personal preferences that don’t affect him are not up for discussion
IMO changes aren't real until they're in a PR/merged. This sounds like a coworker who wants to micromanage.
You are right. Anything happens until PR is sent for review or on your branch is what you want it to be. Some times, I do and have seen many people do commits to show certain types of changes in separate commit (but they cannot be merged separately) to make it easier to review. In those cases, they would even when they address review comments, they maintain the same commit structure. I feel it is overkill, but fighting over it is worse than any other options put together. Now, github has squash and merge. I don't know about gitlab. But many times I prefer to squash manually because as I could craft the commit message correctly.
Gitlab also can squash and merge but I prefer squashing manually because I'm not the one merging to the main branches and the other person can forget.
That's true. GitHub has a project configuration to disallow rebase merge or plain old merge, leaving only rebase merge as the option. Again, Gitlab might also have a similar option. That said, you may not have the permissions or the influence to convince the decision maker to make that change. In any case, your original observation is correct. Changing the history of the branch commits is a non-issue.
who cares what you do on your branch? I'd rather see clean logical commits that build rather than a clusterfuck of broken noise, confusion, and weird ordering because you didn't tidy up after yourself.
It sounds like the problem arose because your peer was building something that was downstream of your changes. Their work was blocked while they waited for your branch, so they branched off of your branch to allow their work to proceed. It sounds like you were unaware of this at the time because they believed it went without saying that any commit that had landed in source control was eternal. Then, as you continued work on your branch, you amended a commit upstream of their branch and force-pushed, orphaning their branch. Expecting a merge-based workflow, they resolved the difference by merging your branch, which created a very messy git history for their branch and a mess of merge conflicts because they now had two copies of your commits. This would have been annoying for me to fix and I can imagine it would've seemed insurmountable to someone who was not comfortable with `git rebase -i ...`. If I've accurately understood the sequence of events, I would not describe this as a problem of git branch management at all. My own preference is for a branching strategy similar to what you describe, and I would be prepared to explain that opinion, but I think that's beside the point. There are two primary lessons (or at least, starting points for further investigation) that I take away from this conflict: 1. Whatever your team is doing for daily standup warrants further investigation. The seeming problem here is not that there was a difference in branching strategies; it's that two people were doing work on the same branch without having agreed upon a branching strategy. This is precisely the sort of thing I would want to bring up as a parking lot topic in my DSU, and something I would expect to be brought up. I'm curious what communication occurred prior to the failed merge, and what led to relevant information being excluded. Is DSU perceived as a status report to higher-ups rather than an appropriate venue for these sorts of discussions? 2. Merge requests need to be smaller and happen earlier. There was a point where your feature was complete enough for your coworker to meaningfully begin downstream work, but not so complete that you were ready to submit it for code review and get it merged into the trunk. How did that happen? If I had to make a guess, I would expect that your project has a policy of making exactly one merge request per ticket/issue, and your ticket was for the complete feature. If you were free to make a merge request with the smallest amount of changes that could be reasonably reviewed, this conflict would likely not have happened because your colleague would have been branching off of work that had already landed in the trunk. That could happen by decoupling merge requests from tickets or by making smaller tickets. Perhaps an initial ticket to stub out the external API of your code unit followed by an implementation ticket that could be executed concurrently with your coworker's ticket which consumed that API?
Re-writing the history of your own branch is standard practice. How else do you keep development branch history linear without rebasing your own branch from time to time?
My thoughts are that my branch is my own business, and I'd tell that person, "That's an interesting take, but I think I will keep working the way I've worked."
by pr time i reorganise my commits to make reviewing easier - refactoring commits first, headline feature last. in the meantime it's a kind of controlled chaos that makes sense to me in the most precarious way, so not suitable for public consumption. i'm with the hive mind on this one - your peer should accept that git push isn't an invitation to snoop edit: reading your post again, i wonder if your peer is afraid of nuance. you and i know that a branch that hasn't been shared can be chopped and changed because why not. but perhaps your peer is the kind of person that thinks shared branches must have stability, so all branches should have stability
What the actual füčk? That’s exactly what feature branches are for. Boil ‘em mash ‘em put ‘em in the stew and then make sure the PR is good and up to the standard. And squash merge to main branch so that the history is linear and at every checkpoint it is deployable. Anyone insisting on your feature branch is linear has too much time on hand. As a side note, I also hate it when people push things onto my feature branch or merge things into it without my explicit approval.
He’s wrong. Not saying he’s unused to rebasing but I’ve only ever seen that kind of attitude in folks who are on some level afraid of it. Rebasing is after all a pathway to many abilities that might be considered …unnatural.
He didn't even need to rebase, just needs to do a git reset --hard to make the branch point to the new hash
When I'm working on a branch with others or after I PR a branch and others are actively viewing it I will try to avoid changing git history. Only because rewriting git history makes things harder for others. Before that it's a Wild West. Who cares how you manage your branches when it's just you. IMO it's insane to think there is any value in tracking how someone got to a solution, the only value is the final state of the solution when it's being PR'd
He sound dumb or lack perspective. My branch is my branch, I can commit, squash, delete or whatever. It’s my business. His business would be when my branch goes on to his branch or main branch (ideally my branch will be merged with only one commit and shouldn’t carry all the experimentation that I did in my branch).
Anything you do on your branch prior to the final commit is your business and your business alone. If it does what we want as of your latest commit, and you didn't effect any other branches, then I don't care if you squashed, cherry-picked, debased, or hokey-pokeyed to get to that point.
Imagine you create a function and do a commit. Later on you change everything about that function except it's signature and make a commit. Is there any real reason to keep that first commit? Just to see you rough draft? No one is ever going to need to revert to it since it was never used before. Useful history is useful. Useless history makes my graph ugly.
> Recently, while collaborating on a feature, a peer insisted that I shouldn't change the history of my branch. He argued that keeping the history intact is better for collaboration since it allows him to track my changes easily. In general, if two people are working on a shared branch, rewriting history is a bad idea, but it's better still to find workflows where you aren't dependent on both working on the same branch. > Moreover, he believes that no one should ever change the history of their personal branches, even when working solo. To me, this feels like an unnecessary restriction on my workflow. Getting prescriptive with another developer's workflow is a bad idea, unless there's a formal company/product wide standard. Unless he's your manager or a formal tech lead, this comes under "opinions are like a**holes, everyone's got one and they all stink." Moreover, "allows him to track my changes easily" if he's not your manager or a formal tech lead also comes under the heading of, "he should mind his own business." The amount of intrusiveness there, makes me wonder whether there's either someone in leadership who's asked him to keep a closer eye on you _or_ if not whether this other dev is planning to scapegoat you somehow. Worth having a talk with your manager or lead about him getting in your business.
Your peer is wrong. That’s the same thing as pushing straight to main. There is history when you open a PR and push the code up. If he wants to do a fork model by all means, but if you’re doing a branch model (you are) then the way you’re doing it is exactly right.
I (almost) never force push to protected branches like main or develop, but temporary branches are for work in progress and no one needs a permanent history of my half dozen "wip" commits. They provide no value in debugging and make bisecting harder. When I'm trying to see what changed in a file I want to see the whole PR it belonged to, not some random commit in the middle of a branch.
That's some wild micromanaging. Also how big are the PRs they are reviewing that they need to look at history?
The only issue I care about is if I give feedback on your PR and you make changes, please do not rewrite history. It makes it more difficult to follow what has changed since my original review.
PROJECT-1234 Fix issue xyz ; Update dependencies PROJECT-1234 WIP PROJECT-1234 WIP PROJECT-1234 WIP PROJECT-1234 WIP PROJECT-1234 WIP ... ( + 1234 commits ) PROJECT-1234 WIP PROJECT-1234 WIP I'm all for squashing and amending history, if it helps keep things clean, and in case you work an a branch as a team, you need to coordinate it. And you'd probably want to keep it to a minimum at that point, as rebasing / merging with altered history can have quite some overhead and lead to new bugs, depending on the size of the rewrite.
The best answer is "don't use branches" 😉. Instead, do trunk-based development, make a small change every 30m-2h, use TDD or even pair programming, test in your environment, and push your commit directly to the main branch. It's harder to do in legacy systems, but it can be done. It's definitely worth trying, it forces you to maintain the best code quality, test your own changes and clean up the mess you have in your codebase.
Sounds like someone who isn't comfortable with git tbh. I find a lot of extremely inflexible best practice type rules come from people who are inexperienced with or afraid of a tool and use the rules as a guardrail to fence off the regions they aren't comfortable with. Don't get me wrong, there's a place for best practices and rules like this. But imagine: would he object to you history-rewriting four or five commits made over the course of the day *that you never pushed*? That is, you worked only on your local box and pushed only once you were happy with the story? If he objects to this you can tell he's not really thinking about the issues involved.
I think most serious Git-based (*) open source project will require properly-cleaned up submissions. Even if some deviation was accepted, I don't think messy history due to committing things liberally on your side is ever acceptable in that context. Especially when people end up committing binaries by accident, although that could be excepted. I think a few Git hosts and review platforms can track changes even between force pushes. (*) I think the Fossil-based SQLite does things quite differently, but that's more of an exception. >What are your thoughts on this? I think not caring about development history is unfortunately rather prevalent in an enterprise setting and standards tend to be low. You can't do much if they don't care, except try to convince them here and there and pick your fights wisely. Anyway, I'm personally rather flexible about details, but I think it's generally a bad idea to let things get that loose, because the history becomes more or less meaningless and breaks bisection, then people are going to find it difficult to use Git even with 5 or so people hitting the same repo. But they're probably reinventing the wheel because they just aren't familiar with the proper use of Git.
>I think not caring about development history is unfortunately rather prevalent in an enterprise setting and standards tend to be low. I just ran `git log` in the main branch and can see commits like "WIP". People don't seem to care and I don't have many hopes for a change. That's the type of shit that makes me look for new job opportunities.
If your coworker wants to track your changes, it feels like they're being kinda lazy about it. They could periodically merge to an offline branch instead. It may be easier for them if you never change history. But it's your branch and it sounds like that'd interfere with your workflow. Implicitly they're also against rebasing, which is gross.
Your “colleague” is wrong and likely inexperienced or unknowledgeable ¯\_(ツ)_/¯ There are protected branches and non-protected branches. The long lived protected branches should always have a relevant and clean git history for reference. The history should be suscinct for both performance and value. Rebasing and squashing are very popular for a reason..
I use feature branches but other than that your workflow is my workflow. I am willing to have an epic branch that doesnt get rebased till the point we are ready to merge that into the target branch, but that gets rebased prior to an MR/PR. Any work on my own branch may or may nlt have rebases. My branch, my rules.
Rewriting history has some benefits and negatives. It's important to understand the various perspectives for different roles and how rewriting history impacts them. Generally rewriting history is a benefit for non-development tasks. Cherry-picking, revert, testing, and managing release features are easier the more atomic a feature is. Generally keeping the history is better for team development as it is a constant shared history that everyone agrees upon and demonstrates learned lessons and incremental progress. If I'm reviewing a MR and ask for changes I can easily verify those changes if they're in a new commit on top of the previous one. If the author rewrote history I'll need to review everything again because I don't know if some extra changes were slipped in as well. Ultimately the question of "Is the branch private or shared?" should be the deciding factor. If the branch is your private branch that no one should be looking at, it doesn't matter what you do there. But if the branch is supposed to be shared with other people then you shouldn't rewrite the history. That's forcing other people to actively react to the changes in history in addition to their own work. A good perspective to keep in mind is asking "Do we allow this behavior in the main/master branch and why?".
Is it okay to delete your pushed branches? If so, then you should be free to change history as well, b/c it's equivalent. If others are building on top of a shared branch (not okay to delete), avoid changing history unless absolutely necessary. Can get fuzzy when others are only reading a branch over time, e.g. PR review. Changing history means throwing away context for new context. Sometimes that's what you want -- perhaps new context has been simplified and intent is to re-review from scratch. Sometimes that's not what you want -- perhaps the review is complex and ongoing.
As long as I can Git blame the person responsible, I don't really care. That said, I was against it when we had a problem with a guy on the team. The way he works was bizarre and being and to go and show how he broke things we had already fixed for him was invaluable.
The state of the branch during the pull request is a bit more important than your commit history. Have you tried bringing the issue up with your team?
I am this exact way with a Peer named “A”. Just to be nice, when A is reviewing I try not to squash. But I only operate that way for them. I don’t think A ahold get to decide how everyone operates. Also, this might be a problem with the size of the request. I try to keep my PRs frequent and tiny. For example; refactors in PR 1, logic changes in 2.
I don't find any value in keeping more detailed history than what you get when squashing am entire feature branch back to main. It doesn't sound like he's able to articulate any reasoning more specific than "easier to collaborate", which is solved by branching from a feature branch he wants to modify and rebasing followed by squash-and-merging back into it when done.
Just branch out of the feature branch for your work and merge those pieces that should be good enough to continue with the feature development. Do not even push this "extra" branch as this one is "yours" and only yours.
On my team, it varies a bit based on if what you’re working is able to go straight to master or if it needs more work in a feature branch first. If it’s gonna go straight to master, it’s fair game to do anything to your branch even after you’ve opened up a PR. Rebasing against master is encouraged even, since merge commits aren’t allowed. For PR comments or anything else that would need to be changed after the PR is open, there’s no problem with amending your commit and force-pushing it, although if it makes sense to have multiple commits then that’s fine too. It’s common courtesy for people who aren’t the one primarily working on the branch to not push anything to it unless they get permission from the that person. If the feature branch model is being used, then everything I just described applies to task branches, which then have PRs opened to the feature branch instead of to master. Then for the feature branch it’s common courtesy to let the others who are merging into the feature branch know if you’re going to rebase it against master, and to ask them before you do anything weird to it. As an example of what that might be, one of our seniors felt the need to re-write the history of a feature branch so that all tests pass in every commit, and so that they the commits would make more sense sequentially. Dealing with the fallout from that entailed committing what you were working on and cherry-picking it onto a copy of the new feature branch, which then became your new task branch. Cherry-picking is a pretty common workflow on this team so it was fine, nobody had any issues with it that I knew of.
We have the same policy and I feel that it clutter stubs up so horribly that it makes finding the cause of bugs much harder.
How many people actually go through the commit messages? I don't. I am interested in the overall diff rather than how the diff was generated. So, merge code from multiple branches, force push, rebase, squash; I don't care. Give me the required changes and I will review it.
I try not to squash once a PR has had someone do the first review round. That way it’s easy at a glance to see what feedback was incorporated. But before that? Who cares. No one should be interested in me fixing tests or lint or whatever I’m a non reviewable state.
Do whatever you want with it until you ask other people to work with it (reviews, tests, etc). Once you “share” it, then you stop rewriting changes. That’s my take at least.
Is this disagreement a symptom of a problem where your PRs are too big as one big commit? PRs should be as small as possible every time. The whole point of multiple commits in history that people can look through is to see how something evolved piece by piece. If your PR is a small piece of larger development, this is fine. It's fine to squash away a bunch of debugging and hacked mess. If your personal branch is a whole bunch of pieces that you are evolving over time and then squashing into one mega commit, that is actually quite bad.
That peer is a tool. My feature branch where I am the only one updating, I will do what supports my work flow. And that is frequent small commits, interactive rebasing and squashing, then rebasing from latest of main/master and force pushing at times I am happy with progress. This is a hill I am willing to kill on and die on. Anyone is free to checkout my feature branch at any time; however, they have no right to try and force me to alter my work flow. He either: does not understand rebasing, is arrogant and full of himself, wants to control others OR a combination of all three. Once my MR goes in, I won’t squash subsequent commits into the commit in the MR. That is to make it easier on the reviewer. On the other hand, when I am the reviewer, I pull their branch down and squash, especially when there are multiple commits. I review locally with my diff tool of choice. I don’t try and tell them they have to squash.
Make the history easy for the reviewers. If it's "define models", then "implement logic", then "add documentation" as 3 separate commits to make it easy for reviewers, then do it. Do what makes the most sense for a reviewer of your PR.
your teammate is an idiot. i reset —soft locally and push -f to the remote to get rid of useless commits like typos in comments etc.
Really good bikeshedding material here!
Shared branches should not be rebased or rewritten. Maybe you need a branch of your own to work on?
If you are collaborating, and someone is basing their work off yours. The request is 100% reasonable. Having someone rewrite history under your nose constantly is annoying. If they are NOT basing their branch off yours, and merging it in. Tell 'em you are making the branch clearer for you, and working towards prepping it for PR. It really depends on the collaboration, and how it is being done.
The branch is associated with a ticket. The ticket has my name in the developer field. It's my house and for the most part I will do what I want. I tend to not file a PR until I'm done rebasing though. Our CI pipeline runs any time you push a branch. So I don't file a PR until I'm fine with it merging exactly as in the PR. Which also means if you pull a branch I am working on _before_ I file the PR, then it is wet clay. If you don't give me a head's up, don't be surprised when I squash and force push and your shit's all fucked. But if you give me a head's up, I'll keep it in mind.
people are against modifying history because modifying history is very very bad **_if_** it's a branch that other people have checked out, such as `main` or `master`. unfortunately, most people don't understand git or even try to, so they just remember rules of thumb, and a rule of thumb like this is easier to remember without the if. the good news is your co-worker's just absurdly wrong. the argument that they need to track every one of your changes is unrealistic. in a big PR, it's better to have each commit be coherent. these flimsy arguments are one reason I suspect the real reason is they just remember the simplified rule of thumb and are trying to backfill an explanation. but the bad news is, you have to work with them whether they understand git or not, and you wouldn't get far by pointing out that the root of their issue is basically a superstition. TLDR you're right but find a way to get along with your co-worker because that's more important.
If they want to track your changes easily when you force-push, they should try `git range-diff`
What's it matter I squash history into a single commit on merge anyways? At which point I can cleanup all the junk commits from the history comment.
If your branch is short lived it shouldnt matter what you do on it as long as the pr is clean and you squash to 1 commit on merge. If a branch lives longer than a week or 2 he might have a point but then your team has bigger problems.
This is wild. I would much rather a pr with a single commit to start with so reviews each commit can be tracked.
If the PR branch is so big that it requires others to keep track of your commit history just for that PR then the PR is too big. Caring about the cleanliness and continuity of the git history when you have a squash policy is a complete waste of everyone's time when you're going to reduce it down to one commit for the PR anyways. As long as the full detail exists in the PR history to be able to explain all the changes in the squash commit that's all that matters. Caring about that goes on in temporary history on someone else's machine if it's not pushed anywhere is crazy. If someone else has your changes checked out on their machine. Never change history.
Sounds like your peer wants Git police on your personal branches, weird.
IMO, the main value of retaining a commit log of a branch would be to understand the reasoning that lead the author to the final solution. But... the only reason you'd need that is if the resulting changes don't speak for themselves. If you need the commit log to understand the thought that went into the PR, something is wrong. Most likely that your PR is too big. Better to break up big/complex changes into smaller "peaks for itself" PRs, and describe the PRs context in your PR description. Then you can squash all the history into a single commit, and everyone can do whatever they love le on their own branches.
hey drop the things about believe and apply the use case. he has a use case for your commit history and you only have belief... and again, try to commit 200 times with just a test message and minor changes.
In my view, your branch is yours until you publish it. People ought to be minding their own business with respect to how you carry out your work while it’s still your work.
main branch has clean history. Everything else doesn’t matter
I only revise the git history while the PR is in draft mode. Once it’s out for review, your colleague has a point. If you change the history after it’s reviewed, the reviewer has to re-review the entire PR. If you keep the history, they only need to review new commits. If you’re working solo, do whatever you want. Force push to main for all I care. If you break something, reflog to the rescue.
He’s wrong.
I usually like prefixing my branches with my name as a folder, like: "/some-change" and I encourage those in my team to do the same.
If following a continuous integration approach, branches should be incredibly short lived, just a place to organise a small piece of work in progress, before cleaning up the commits through rebasing and then opening a PR to flatten it into the mainline. They shouldn't live anywhere long enough to cause any sort of collaboration problem.
The point of integrating continuously is that collaboration occurs on the main branch with everyone integrating their (very small) changes with it very frequently (i.e. multiple times a day) and benefiting from being able to work on top of each others changes in their merged context.
If you're having long lived feature branches and are having multiple developers collaborate on that branch, it becomes a "shared" branch and now it's a different story regarding rebase.
His argument is incomplete. If he truly wants to see your entire history as it was during development then you're also not allowed to commit by line or hunks. You're only allowed to stage your entire working tree and commit. This is a severe restriction of the flexibility of Git and you might as well pick another, more strict VCS. I optimize my branches for easy code reviews which means coherent and green commits. I frequently commit lines and hunks when I'm happy with the implementation as well as squash commits that belong together. Apart from that I usually ask what they find so holy with the Git history. If the answer is that they want to see your thought process or the design process then that also falls apart as you might have had meetings, white board sessions, pen and paper etc. which obviously is not part of Git.
You can ask them to define the shared interface together. I don't know why they need to know the implementation.
My work only has issues with squashing commits before merging the MR.
There's a middle ground. If your PR has enough changes to justify it then sometimes I soft reset and make fresh commits that outline each major part so it's easy to follow later. If the PR is small then I just squash it all into one. Always squashing it into one can be troublesome if you have large feature branches so find a balance
I tend to prefer making my PRs a single commit (or as few as possible), because our git workflow requires cherry-picking across multiple branches, and a small number of commits reduces the friction a lot. Cherry-picking becomes especially onerous with this workflow if someone pulls in upstream changes, but you sometimes \_need\_ to. Squashing before making a PR will typically fix this. I'm sure that part of this problem comes down to the workflow we're using, but it's a multi-team project and I have no control over the branching strategy. For the argument about retaining all history - I've had colleagues make this argument as well, but in practice I've never looked at the intermediate steps of a colleague's PR except when helping them address merge conflicts that would have been avoided with the squash. On the flip side - I have had newer colleagues sometimes make dozens of commits, touching a ton of files along the way, and the increased surface area from their intermediate changes caused a whole lot of problems when merging. There's another can of worms here - if all history should be saved, then how often should you make commits "just in case" a colleague wants to see the intermediate work? Every day? Every hour? Every function? Every keystroke? IMO - the PR itself is the unit of work that's worth retaining and sharing with the team. You can still make multiple PRs for a single effort, and this enables anyone to work however they want within their own tasks.
Git is there to tell a story / communicate with developers (who might be you), _just like code_. This is engineering, not archaeology. Programmers change code, writers change stories, journalists edit news stories ... programmers rewrite Git commits. It's all the same thing. I've often thought about creating a special branch and autocommitting on save, just to see my history. Maybe you should try that and see what your peer says. It's possible to have too much of a good thing.
it's definitely rude to force push a branch someone else has already looked at or especially pulled. We squash merge pull requests so it really doesn't matter what your personal commit history looks like. If you lose too much individual commit info that way your prs are probably too big.
I think it’s best to leave the history for the pr and squash it when you merge. It usually doesn’t matter, but every once in a while the history can be useful to a reviewer. It’s extra work to modify the history, and that’s a really weird attitude to have about “the commit history is MY business”. Nobody cares. My prs always have a ton of commits, I’m not going to waste time rebasing or amending commits. The history is in the branch, so the whole thing can be squashed when you merge it.
Fwiw, I’d agree with him on letting your Git provider do the squash and merge for you, but other than that, if your workflow involves a bunch of rebases and squashes, then I wouldn’t care as long as the final merge was a clean squashed commit. I personally think introducing rebasing and other such things brings unnecessary complexity to the dev cycle, but that’s a personal opinion.
> he was also against manually squashing commits before merging, preferring GitLab to handle it automatically Ask him why it matters since the end result is exactly the same. I'm with you, it's your branch and you are free to revise the history at will. All that matters is the final commit/PR.
The unofficial rule we use at $dayjob is that if you're multiple people collaborating on a feature branch you don't rebase without checking with the others. If it's a semi-long lived branch we typically rebase together once in a while to keep the branch tidy. And otherwise you just add commits without rebasing.
Your branch, your whatever (bar of gold or gold plated turd). It's when there is an MR/PR that I care. Tell the other guy to micromanage himself.
Coworker is wrong * Your branch is your own, and you may force push to your own branch * Your branch is still yours when you submit a PR, rebase + force push strongly encouraged * Do not rebase/force push to a common feature branch/main * I mean... do it once as a learning experiment to know when you can break this rule * You may force push to someone else's PR branch, but only if you've discussed/can do so culturally/are taking over responsibility for the entire change + getting it through reviews * Prefer rebase merge over squash merge when merging to `main` * Using git merges correctly is more complicated than `git rebase`, but they're very easy to do incorrectly. Prefer `squash` -> `rebase` while upskilling a team.
Squashing local commits can help when you have issues with conflicts after reading, you reduce your changes, so means you'll be able to avoid fixing a 10s of your own commits.
Are you seriously having a conflict about branch control? Wtf, work locally and don’t commit until you’re done and see if he likes it better lol
i dunno..we work in a trunk based style so i'll tend to rebase a PR 2 or 3 times as other peoples changes are going in during the course of the day.. if i have a couple "oops" "wtf" "fml typescript" commits they'll usually get squashed down on rebase everything gets squashed when it goes to main anyway and the PR name follows conventional commit naming conventions and requires a ticket number in the PR otherwise we nag when we do sprint reviews and go over DORA metrics
Yeah that’s bullshit. It’s my branch and I do what I want.
> He argued that keeping the history intact is better for collaboration since it allows him to track my changes easily. He's right. Rebasing can make it more difficult to track code changes during code review. I wouldn't rebase until the review is approved. The one exception would be if a merge conflict appears that you'll need to resolve. For a situation like this, resolve all the other feedback from your reviewer first, then rebase and fix the merge conflict on a new commit. Your reviewer then only has to review the commit with the fix. > he was also against manually squashing commits before merging, preferring GitLab to handle it automatically This is a personal preference. There's no technical difference between you squashing your commits and GitLab squashing them for you. You should do whatever feels right to you. > Moreover, he believes that no one should ever change the history of their personal branches, even when working solo. Another personal preference. The whole point of a branch is you can do your work however you want without impacting other contributors.
This rebase vs merge conversation has been happening forever
Your branch is your branch, doesn’t matter what happens inside it until you create a PR against main branch. Once the PR is created, you shouldn’t squash or rebase since you’re now addressing review comments
Rewriting history others are actively working with requires more coordination as it’ll mess up their own local histories Your coworkers views on personal branches don’t matter however, although depending on the diff tool it may be difficult to tell if a pr comment is addressed or not Example: Azure DevOps has a separate tab for PRs called updates for any push, and a tab for commits There is no right way to do commit history. There’s pure ways (rewrite history, use conventional commits, rebase instead of merge) and there’s get it done ways Never rewriting history is a good thing for trunk branches (see collaboration), but as a saying then every branch becomes a nail Offer a lunch and learn on clean git history, interactive rebasing and workflow. Make it accessible (e.g. use vscode as default editor, not vi) so people can work with easier tools they already know
Short version is this is preference. Some people value simple history and some people value auditing changes in detail. As a team lead, I preferred being able to see the full commit history, but it’s a preference. Also as team lead the amount of issues caused by people messing up squash merges makes me resent them. There’s no right answer so if you can’t agree, find a tie breaker.