6 ms·

> How do you git bisect a codebase where every 6th commit doesn’t build because it was jammed into the middle of a Pull Request? This I think is the main probl

> How do you git bisect a codebase where every 6th commit doesn’t build because it was jammed into the middle of a Pull Request? This I think is the main problem with pull requests. A better default would be squashing the commits into a single one after review.

pbh101 8y ago
This is also suboptimal. I’d rather each commit be an atomic unit: no larger and no smaller than it needs to be. Edit: in other words, concurring with TFA.
- ghayes 8y ago
  Sure, but the best way to get there in my mind is `rebase -i`, which means you've made a few and revised a few commits along the way.
  - deleted 8y ago
    [deleted]
- TomK32 8y ago
  yes, but often on a feature branch you have WIP, change your code a few commits later and so on. I use rebase -i a lot and it make better code.
  - Sean1708 8y ago
    `rebase -i` is fine (`commit --fixup` and `rebase --autosquash -i` is better), squashing all commits down into one is not (at least doing it all the time is not).
falsedan 8y ago
You can start bisecting and then skip every commit on a feature branch that was merged in. Painful to script and I wish bisect had a --first-parent flag to do it for me
Joky 8y ago
Bisect on first-parent? Seems like a script exists (haven't tried): https://gist.github.com/ayust/2040290 https://gist.github.com/ayust/2040290
- eequah9L 8y ago
  Wouldn't this mean you just find the merge that caused the regression? The merge itself might bring in an awful lot of changes (of a single topic).
  - zbentley 8y ago
    I think that’s desirable. First and foremost, you want to know whose work (in this case, a merged branch) broke yours. Afterwards you might want to drill into what specific piece (in this case commit) of that work was responsible, which can be achieved by bisecting the branch’s non-auto-merge/non-rebase history; this usually gets you close enough to “where the bug was actually written” in my experience, without requiring a switch to a specific workflow of what a commit is supposed to represent/how committed code should behave.
  - Joky 8y ago
    The merge won't bring more changes than if the PR was squashed in a single commit though. Hopefully if you have a test broken and the PR merged two commits: one that refactored the code and the second one that leveraged the refector to change the behavior, you can easily try to revert the latter to check if it fixes the problem. The commit is smaller, easier to re-review, and the bug likely easier to spot.
kzrdude 8y ago
As others have pointed out indirectly, git bisect is designed with the patch stack in mind and could be made to support pull requests better.
donatj 8y ago
I have always been of the mind that every commit should be a working state. My coworkers who often commit with the message “WIP” disagree. I doubt I’ll ever sell them on it.
- lima 8y ago
  Yeah...WIP commits are fine in your local feature branch. I wouldn't ever want to see a WIP commit in my origin/master.
- pdpi 8y ago
  The stacked diff approach says: You get to have both, because it pushes you towards a rebase-heavy workflow, where you keep updating the WIP commits until they're ready to merge. This is a pain in the arse with git, but `hg absorb` makes it 100% painless
- ajuc 8y ago
  WIP commits in gerrit are perfectly fine. You can even push them to gerrit with all the rest commits. Just filter "WIP.*" out in gerrit view that lists commits to review.

pbh101 8y ago

This is also suboptimal. I’d rather each commit be an atomic unit: no larger and no smaller than it needs to be. Edit: in other words, concurring with TFA.

ghayes 8y ago
Sure, but the best way to get there in my mind is `rebase -i`, which means you've made a few and revised a few commits along the way.
- deleted 8y ago
  [deleted]
TomK32 8y ago
yes, but often on a feature branch you have WIP, change your code a few commits later and so on. I use rebase -i a lot and it make better code.
- Sean1708 8y ago
  `rebase -i` is fine (`commit --fixup` and `rebase --autosquash -i` is better), squashing all commits down into one is not (at least doing it all the time is not).

falsedan 8y ago

You can start bisecting and then skip every commit on a feature branch that was merged in. Painful to script and I wish bisect had a --first-parent flag to do it for me

Joky 8y ago

Bisect on first-parent? Seems like a script exists (haven't tried): https://gist.github.com/ayust/2040290 https://gist.github.com/ayust/2040290

eequah9L 8y ago
Wouldn't this mean you just find the merge that caused the regression? The merge itself might bring in an awful lot of changes (of a single topic).
- zbentley 8y ago
  I think that’s desirable. First and foremost, you want to know whose work (in this case, a merged branch) broke yours. Afterwards you might want to drill into what specific piece (in this case commit) of that work was responsible, which can be achieved by bisecting the branch’s non-auto-merge/non-rebase history; this usually gets you close enough to “where the bug was actually written” in my experience, without requiring a switch to a specific workflow of what a commit is supposed to represent/how committed code should behave.
- Joky 8y ago
  The merge won't bring more changes than if the PR was squashed in a single commit though. Hopefully if you have a test broken and the PR merged two commits: one that refactored the code and the second one that leveraged the refector to change the behavior, you can easily try to revert the latter to check if it fixes the problem. The commit is smaller, easier to re-review, and the bug likely easier to spot.

kzrdude 8y ago

As others have pointed out indirectly, git bisect is designed with the patch stack in mind and could be made to support pull requests better.

donatj 8y ago

I have always been of the mind that every commit should be a working state. My coworkers who often commit with the message “WIP” disagree. I doubt I’ll ever sell them on it.

lima 8y ago
Yeah...WIP commits are fine in your local feature branch. I wouldn't ever want to see a WIP commit in my origin/master.
pdpi 8y ago
The stacked diff approach says: You get to have both, because it pushes you towards a rebase-heavy workflow, where you keep updating the WIP commits until they're ready to merge. This is a pain in the arse with git, but `hg absorb` makes it 100% painless
ajuc 8y ago
WIP commits in gerrit are perfectly fine. You can even push them to gerrit with all the rest commits. Just filter "WIP.*" out in gerrit view that lists commits to review.