Skip to content

Bisect improvements#36

Merged
andrewbranch merged 2 commits intomasterfrom
bisect-improvements
Sep 12, 2022
Merged

Bisect improvements#36
andrewbranch merged 2 commits intomasterfrom
bisect-improvements

Conversation

@andrewbranch
Copy link
Member

@andrewbranch andrewbranch commented Sep 12, 2022

Change described microsoft/TypeScript#50670 (comment):

The issue with the bisect was that the culprit commit was cherry-picked from main to release-4.5, and I only look at the timeline of commits going into main. I’m far from a git expert, but (warning: likely butchering terminology ahead) git bisect gets mad if you give it a pair like v4.4.4 and v4.5.5 because those tags exist on separate branches off of main that never reunite with main. To account for that, I actually run the bisect between the merge-base of the respective inputs and main, so I’m actually bisecting between the point where release-4.4 and release-4.5 branched off of main. But in hindsight I think I only need to take the merge-base of the old ref. There still might be a problem if the behavior is different between the old ref and its merge base with main, though. I think that should be less common.

This also changes the GH Action API a bit; TypeScript PR incoming.

@jakebailey
Copy link
Member

I do bisects between tags on different branches all time time when bisecting by hand; it just starts by testing the merge base first automatically and bails out if that commit doesn't have the "old"/"good" behavior. (Maybe I'm confused and that's what you're reimplementing in TS?)

@andrewbranch
Copy link
Member Author

Hm, I had some kind of trouble with it when I was first setting this up. Don’t recall exactly how.

@andrewbranch andrewbranch merged commit e19e628 into master Sep 12, 2022
@andrewbranch andrewbranch deleted the bisect-improvements branch September 12, 2022 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants