6 ms·
Let me try again. I'm advocating that you use rebase to improve the quality of your changes that will be reviewed before merging or even before being reviewed a
by gwright 7y ago
Let me try again. I'm advocating that you use rebase to improve the quality of your changes that will be reviewed before merging or even before being reviewed at all.
If I make three commits and then realize that I should have included something in the first commit, I use rebase to create a new sequence of three commits that has the corrected version of the first commit. I haven't shared those commits with anyone, this is just work that I've done locally.
Are you seriously advocating that creating a pull request with: (A, B, C, A-fixup) is better than using rebase and then creating a pull request with: (better-A, B, C)?
You think that second case is "lying" because I didn't show the intermediate step that included the mistake?
- Nullabillity 7y agoYes, it is lying. You can mitigate most of the damage if it is convincing enough (for example, go through B' and C' and make sure everything still makes sense at each point), but realistically nobody is going to do that, because it's pretty inefficient way to spend your time. And even then, you're still removing context (unless you're just fixing a typo). > I'm advocating that you use rebase to improve the quality of your changes that will be reviewed before merging or even before being reviewed at all. That was clear from the start. But the fact that X breaks Y doesn't imply that Y is a good idea when X doesn't apply.
- gwright 7y agoI really don't understand why you are choosing to use the word "lying". Us mere humans make mistakes all the time. Typos, omissions, false starts, and so on. What is the value of throwing that raw set of events at a reviewer or complicating the understanding of the changes when viewed in retrospect from the future? What is the reason you call curating the work into a more polished form "lying"? Why do you think the time spent being intentional about changes isn't valuable when compared to the time spent by a reviewer (or your future self) to sort through the flotsam and jetsam of your intermediate work?
- Nullabillity 7y agoI'm using it to differentiate between summarizing (removing steps between A and B) and modifying (introducing new steps, reordering them, or editing them). You can do it in a way that isn't harmful (as I mentioned earlier), but good luck getting a team to actually stick to that. It also doesn't help that pretty much no tooling encourages doing it properly.
- avar 7y agoAre you lying to your co-workers when you draft an E-Mail to them, read it over, and decide to delete a paragraph or write it again from scratch? If your E-Mail client automatically saves drafts that's basically the equivalent of "rebase". I made a typo when writing this reply, and pressed backspace to correct it. Is use of the backspace key lying? I think you're placing a value on "history" that doesn't map onto all users of "rebase", or E-Mail client drafts. A lot of advanced users use it as the equivalent of "save" in an editor, sharing all those intermediate states is more noise than value v.s. crafting a sensible patch once you figure out what you want/what change to make.
- Nullabillity 7y agoNo, if you only merge changes then you're just summarizing. The true problems begin once you start creating commits that represent repository trees that you never tested or reviewed, for example by editing past commits (invalidating any testing you've done of commits after that point), deleting past commits (aside from squashing an unbroken sequence of commits, or deleting them if the squash would result in a no-op), reordering commits, or rebasing commits.
- avar 7y agoYou're assuming that commits are tested before they're made, and that rebase invalidates this. I don't test most of my commits, just like I don't proofread an E-Mail after every word I've written. I do that later. But yeah, the history you push to a canonical branch should generally be made up of commits that have all been tested in isolation. The rebase command doesn't make this worse, but better, e.g. with "rebase -i --exec='make test'". I also prune out history of some false steps taken. Have you never written a program and done something like "I'll use a hash here <save><compile><test>, no actually a list makes more sense <save><compile><test> ...". Those intermediate steps are commits for a lot of advanced git users. Sharing all your mistakes-as-you-go-along with the world doesn't help anyone, I'd typically be sending you a 100 patch merge request for some rather trivial change instead of 1-3 sensible commits.
- baddox 7y agoMaybe "lying" isn't the best term to use here, because it seems like you're using it to mean "not providing all information in a way that is morally bad." Of course you're not providing literally all information. Heck, I conceal a lot of information about my development process by testing and changing code before I even make a commit. But I think that's preferable to, for instance, providing a video screen capture of my entire development process for review.
- aldanor 7y agoYea. Moreover, I would often ask my coworkers to rebase their code if there’s commits like “oops, missed a comma” because it distracts from the main point when you read the commit history.
- bonzini 7y ago> realistically nobody is going to do that, because it's pretty inefficient way to spend your time. Of course you do! And it's not am inefficient use of your time, because it helps reviewers now, and yourself when you're bisecting later. > And even then, you're still removing context (unless you're just fixing a typo). You place that context in the commit message.
- fulafel 7y agoIME this is a very rare level of sophistication in use of rebase. And how do you detect that you forgot / was too busy to do it, when you go back 6 months later? It's fragile, "fail-open".
- bonzini 7y agoYou would be surprised. For example, this is a guide my colleague wrote to describe his git workflow: https://github.com/tianocore/tianocore.github.io/wiki/Laszlo's-unkempt-git-guide-for-edk2-contributors-and-maintainers https://github.com/tianocore/tianocore.github.io/wiki/Laszlo...
- dang 7y agoThrowing the word "lying" into an argument like this counts as name-calling and flamebait in the sense that the site guidelines use these terms. It leads to distracting, shallow, and therefore more boring conversation. Would you mind reviewing the rules and please not do that? Let's stay focused on exchanging what we're curious about. https://news.ycombinator.com/newsguidelines.html https://news.ycombinator.com/newsguidelines.html