3 ms·

> What you're proposing doesn't actually improve matters; you don't gain anything (other than saving a bit of machine time) by only testing known-compiling comm

• lmm 6y ago
  > You're not being quite honest here: of course just knowing if something compiles at all, whilst nice, is not that massive a difference (because you can fairly easily script an automated bisect that will skip non-compiling commits by itself). But that's much weaker than what I actually said: knowing that the commit has passed your (1.5 h) CI, and making it so that all your merge commits on master will have this property. In the (somewhat exceptional) case I'm talking about it was 1.5h for a standard build; IIRC straight compilation was more than half of it. In any case, there's no real difference between compilation and "CI" here; you make your bisect script run whatever your CI test is, and skip if it fails, before running the part you're actually testing. > Since you apparently haven't tried it, maybe you should be a bit more careful about dismissing the utility out of hand? I've tried workflows that made it easy to do a bisect that tests only commits from the history of master; unless you can explain how what you're suggesting achieves something better than that, I don't think not having used your precise script invalidates my views. In fact it seems to me that your workflow is distinctly worsened by using rebase; if you used merges then some intermediate commits from feature branches would have been successfully built by CI (pre-merge builds, builds of "early review" PRs that were reworked before merging to master, builds that the developer deliberately ran on CI for whatever reason), whereas by using rebase you guarantee that only the post-merge states of master are available to you for bisection. > if you hit a commit that's broken in a more subtle fashion than "does not compile" but that CI would have found it's not always cheap to work out if you hit the regression or some unrelated temporary breakage. What are you doing on CI that's so different to what you're doing during local development? If breakages don't show up until you make the PR that you want to merge that's bad for everyone; running the unit tests that pertain to the code you're working on, if not the whole suite, before committing is just common sense. Of course it's possible for something to work locally and break on CI, but that's a very rare case (much rarer than generally-flaky tests, IME). > You are also making a several implicit assumptions which are unlikely to hold: it is not true that typically none of the utility of bisect manifests before you have identified the precise lines of code that caused an issue and understood them. If you have some acute problem in production, being able to reliably and quickly locate the feature branch that introduced it is very valuable (for getting the right people to look at the problem or even shipping some work around before the problem is fully diagnosed). Often certainly a recent deployment is to blame. But sometimes a deployment can exercise something that regressed much earlier and trigger a data corruption that is not noticed immediately. Narrowing it down to a branch really isn't much quicker than narrowing it down to a commit - you've already come up with the test case/script, so it's just a case of letting it run for maybe 5 more cases (if we assume maybe 30 commits on the feature branch). If you have a very small team then I guess narrowing it down to a specific branch might be much quicker than narrowing it down to a commit within that branch - but in that case the bisect is going to find the same thing, you can see when it's got to the stage of testing commits from the same branch and start investigating there. And if you really want to bisect just via the history of master, you can always do that (admittedly with a little scripting, but you don't seem to be shy of that). > Also, of course once you found some problematic merge there is no reason you wouldn't then continue to use bisect to find the precise commit in the feature branch! But you can't do that if you've rebased the branch, because most of the branch history is (often) broken. If you were squash-merging you could dig out the "original" version of the branch (assuming it's not been gced) and bisect there (assuming the problem is solely due to a change on that branch and not an interaction between that branch and a concurrent change on master), but if you're rebasing you can't even do that, because if developers are in the habit of rebasing then the "original" branch was probably rebased and force-pushed as well, so is likely to have old commits that don't compile. > Assuming each developer lands something on master every two days on average, you'd have to deal with 250 merges a day. Say you can run about 5 sequential CI runs per work day, you'd need to test pretty large batches of ~50PRs per batch, which is more than I have experience with. Since you will only batch stuff that passed branch CI, and disregarding flakes for the moment, a batch should only fail because of an incompatibility of PRs within the same batch or against some very recent addition to master. So you can probably get the failure rate into the low single digit percentages, at which you'd need to do run enough alternative batches in parallel to cope with one or two bad PRs per batch. That still seems feasible if you put batches together intelligently Yeah, that kind of approach definitely seems viable - we had a design like that sketched out, and may even have prototyped it at one point. It was decided that it wasn't worthwhile, because we would still have needed our process for catching when flaky tests were introduced, and that process caught conflicts like this "for free". So you are probably right that it is achievable to avoid ever merging non-compiling code to master except when meta problems happen, but I don't think that actually takes you significantly closer to "master is never broken". > Sure. But minimizing those bad commits (and probably even marking them after the discovery, e.g. via git-notes) pays off. Yes and no; as long as they're isolated, I don't think pushing the rate of bad commits from 1% down to 0.5% or even 0.1% is really a game-changer. Obviously there's a threshold where you have enough bad commits that it significantly disrupts being able to bisect at all, but unless you can manage to completely eliminate the possibility of a broken commit, there's a wide spread of bad-commit rates where your workflow has to be pretty much the same. Just like how buying higher quality hardware to reduce defect rates is generally not worthwhile unless you go all the way to buying a mainframe with fully redundant everything and guaranteed uptime; you still need to be fault-tolerant if you buy the pro component with 99% uptime, so you might as well buy the consumer version with 98% uptime if it's even 5% cheaper.