Hacker News new | past | comments | ask | show | jobs | submit login

How do you manage stacks of commits for code review? (I'm guessing you just don't -- a lot of git users have really messy, unprofessional repository histories. The fact that git doesn't have great tooling for history management is also an embarrassment to our profession)



As I stated in the comment you are replying to:

> Not that I don't do interactive rebases sometimes in local branches

I don't have a problem with rebasing in local branches before code review. I just don't allow rebasing once a branch has started code review, and I especially don't allow rebasing in a public integration branch ever.

(I also rarely feel the need to rebase, personally, even in my own local branches. Working in better tools than git in a past life has given me a lot of the discipline I feel that I need to build good commits as I work. I make much more use of the git staging area than I see most devs generally do and use `git add --patch` and `git add --interactive` far more than `git rebase`. `git add --patch` especially is an under-sung hero that I think a lot of junior developers should learn well in advance of rebase.)

Also, I'm just not that bothered by messy histories in code reviews when I review other developers work. If the idea comes across even if the commit messages don't have the greatest narrative flow or are perfect, I'm perfectly fine with that. I make sure all merges are --no-ff and after that I can browse a "clean" history post code reviews with DAG traversal tools such as --first-parent. But that also gives me an opportunity to dig back into the mess months or years later after the code review if I need to research a bug or a regression. That sort of "sewage archeology" isn't "fun" by any means, but the number of problems I've been able to solve with it is high enough I appreciate keeping that mess around in source control. It's source control's job to keep the mess for me and present me pretty views for more regular work. That's why I prefer to trust tools like --no-ff and --first-parent over --squash, because that's what source control is built to do, control old histories messy or not.


What can I say. You're locked into a significantly worse maximum than is possible (and exists) using better tools. In a normal workflow your commits are small enough that you don't have to do "sewage archeology".


I don't think you are reading me correctly. That is a rare occurrence, but it is possible when using merge commits to correctly describe the DAG. It's entirely impossible if you overly rebase/squash. Those rare times that I've needed to do it it have "saved" a project a lot of time from a major bug or regression. My normal workflow I look at a "clean" faux linear view, but it's just the "--first-parent" view of merge commits, which gives me a "PR focused" or "integration focused" view of source control first.


If your average commit size is 30-40 lines of code, this isn't an issue. Stacks of small commits is a time-tested workflow, used by the Linux kernel among many other projects.


I think we are talking about different things at this point, but I may be losing the thread of your argument here.

I love a code review to consist of lots of small commits. I'm mostly fine if even some of them from developers more junior than me are just named "commit" if they are small.

When I finish the code review I prefer to only `git merge --no-ff`. All those small commits stay in the code history in the exact form they were reviewed under.

If I'm looking at history I'm most often using something like `git log --first-parent`. In my --no-ff integration branch that just shows me an integrated change list (PR list). Git gives me a "straight line" view down the DAG and hides irrelevant information I don't want in that moment like all the small "commit" commits.

If for some reason I find a bug or issue in some code, I may need to drill down into specific small commits including the "commit" commits, and I have that ability because all those commits are still in the DAG.




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

Search: