4 ms·

Because the evolution of the topic branch is largely irrelevant to future analysis. If you write tests firsts, then you have a commit in the repo where tests c

Because the evolution of the topic branch is largely irrelevant to future analysis. If you write tests firsts, then you have a commit in the repo where tests can never pass; fun for bisecting. If you write the code first, then you have a commit with a bunch of dead code in it because the tests appear in the next commit. I would say that my average topic branch looks like: "edit the protos and rebuild them", "add feature", "add tests", "add CLI", "fix lint". There is no value in checking that into the main history in that order; the first commit doesn't compile because the API servers don't implement the API server interface anymore. The second commit is a bunch of dead code. The third commit is somewhat useful except the code doesn't follow the style guide and static checks fail. The fourth commit at least un-deadifies the code; something calls it now. The last commit is something that can be checked in. I really see no value in preserving that history; the set of commits makes more sense as one commit. You could redo this to "edit the protos, and rebuild them, and add stubs that return Unimplemented", "add feature and tests to replace the stubs that return Unimplemented", "add CLI binding", which could stand alone if you felt like it. It is more work for the author, though, because you had to write code that you are literally going to delete in the next commit. Why bother? Personally, when I'm reviewing code, the more context I have, the better. If there is a design doc for the feature and someone sends me just the protos that they describe in the design doc and the design doc is actually good, fine. I got the context and this just speeds things along. If there isn't, how do I know this API is even right? If I see how it's implemented across the client and the server, then it's easy to make suggestions for improvement. If it's just the API, it sounds right, but I don't know if you can actually implement that feature until I see it done, right? More importantly, how do I know this set of 3 patches isn't going to be followed by "oops, forgot this in the protos", "oops, forgot this in the server", "oops, forgot this in the tests", and "oops, forgot this in the CLI"? Now we've turned one commit into 8. Why? Because "small PRs are good"? Prove it. I had to do 8 code reviews instead of 1. You had to write code that you're literally deleting the commit after you added it. Why bother? I'm unconvinced that the history of "do it", "oops fix it" is more useful when doing archaeology than "do it right the first time".

jkubicek 3y ago
> You had to write code that you're literally deleting the commit after you added it. Why bother? I'm unconvinced that the history of "do it", "oops fix it" is more useful when doing archaeology than "do it right the first time". My suspicion is that people are using rebasing to clean up their prior commits as they go instead of just getting everything working and then breaking the work up into commits, if needed. And this is also how they work themselves into horrible knots trying to figure out what they did to hose their branch.
yencabulator 3y ago
This is a problem with how you work. You're not making atomic commits, you're spreading a logical change over multiple commits. Non-atomic commits are just bad. Here's an example of a correct way to use small commits: add a new function+unit tests into a library in one commit, then switch existing code to use it in one or more follow-up commits.
- jkubicek 3y ago
  How do you correct the following commit history? 1. New function & unit tests 2. Switch existing code to use new function 3. Whoops, new function broke some existing code 4. I guess I need to fix tests too 5. Spelin mistakes There's at least three ways to clean this up: First, you could maybe do an interactive rebase, move commits 3-5 up and squash into commit 1. Maybe easy, or maybe you hose yourself and break everything. Second, just squash it into a single commit. No possible way to screw this up, works every time. Third, do a `merge --no-commit`, commit your changes and your tests, commit the changes to the existing library to use your new method. Easy and straightforward and keeps the two change separate if that's something you're into. Of the three choices, rebasing is the worst!
  - yencabulator 3y ago
    1. git commit lib/bar.x 2. git commit foo.x 3. git commit --fixup sha_of_1 4. git commit --fixup sha_of_1 5. git commit --fixup whichever_is_appropriate n. git rebase -i where_ever_you_branched_off You call git rebase "worst" purely because you don't currently see the value in granular commit history. If and when you do, git rebase is just the tool for the job. `git merge --no-commit` only works for trivial things, real world is a lot messier than that. But perhaps you haven't needed to deal with the complex changes. For an idea of what complex changes look like, skim some linux kernel VFS or MM refactoring patch sets.

jkubicek 3y ago

> You had to write code that you're literally deleting the commit after you added it. Why bother? I'm unconvinced that the history of "do it", "oops fix it" is more useful when doing archaeology than "do it right the first time". My suspicion is that people are using rebasing to clean up their prior commits as they go instead of just getting everything working and then breaking the work up into commits, if needed. And this is also how they work themselves into horrible knots trying to figure out what they did to hose their branch.

yencabulator 3y ago

This is a problem with how you work. You're not making atomic commits, you're spreading a logical change over multiple commits. Non-atomic commits are just bad. Here's an example of a correct way to use small commits: add a new function+unit tests into a library in one commit, then switch existing code to use it in one or more follow-up commits.

jkubicek 3y ago
How do you correct the following commit history? 1. New function & unit tests 2. Switch existing code to use new function 3. Whoops, new function broke some existing code 4. I guess I need to fix tests too 5. Spelin mistakes There's at least three ways to clean this up: First, you could maybe do an interactive rebase, move commits 3-5 up and squash into commit 1. Maybe easy, or maybe you hose yourself and break everything. Second, just squash it into a single commit. No possible way to screw this up, works every time. Third, do a `merge --no-commit`, commit your changes and your tests, commit the changes to the existing library to use your new method. Easy and straightforward and keeps the two change separate if that's something you're into. Of the three choices, rebasing is the worst!
- yencabulator 3y ago
  1. git commit lib/bar.x 2. git commit foo.x 3. git commit --fixup sha_of_1 4. git commit --fixup sha_of_1 5. git commit --fixup whichever_is_appropriate n. git rebase -i where_ever_you_branched_off You call git rebase "worst" purely because you don't currently see the value in granular commit history. If and when you do, git rebase is just the tool for the job. `git merge --no-commit` only works for trivial things, real world is a lot messier than that. But perhaps you haven't needed to deal with the complex changes. For an idea of what complex changes look like, skim some linux kernel VFS or MM refactoring patch sets.