Refactor bundle buddy to be more re-usable in fluidframework #3207
Conversation
this will be added back later in a different location (bundle-size-tools should be reusable; build-tools/bundleSizeAnalysis is fluidframework's wrapper for our own uses)
heliocliu
changed the title
ghost
added
the
make the fallback optional and allow it to be user defined
small issues
| * prior to the previous commit. | ||
| */ | ||
| public static * naiveFallbackCommitGenerator(previousCommit: string): Generator<string> { | ||
| for (let i = 0; i < 5; i++) { |
There was a problem hiding this comment.
no real reason, seemed like a reasonable number that would catch most cases for us without going overboard
There was a problem hiding this comment.
Feels like it shouldn't be limited, except until getPriorCommit does not have a prior commit.
| @@ -11,3 +11,7 @@ import { execSync } from 'child_process'; | |||
| export function getBaselineCommit(): string { | |||
| return execSync('git merge-base origin/main HEAD').toString().trim(); | |||
| } | |||
|
|
|||
| export function getPriorCommit(baseCommit: string): string { | |||
| return execSync(`git log --pretty=format:"%H" -1 ${baseCommit}~1`).toString().trim(); | |||
There was a problem hiding this comment.
It will fail if baseCommit does not have a parent commit. Maybe that's fine? But it feels like it should be a handled case or return undefined or something.
The way it is used in the generator function, I would expect it to make the Generator be done() as true once this case is hit.
There was a problem hiding this comment.
i think it's probably fine for the usage here. if the command fails this function will still return a (garbage) string which should manifest later as being unable to find a build for a garbage commit (which is logged to console)
fix generator usage
add option to change defualt number of builds to search. potentially useful for us because we have a high number of builds (especially if you haven't merged master very recently). also change it to only pull builds once so we don't hit the ado api a bunch when doing fallback
Refactor our copy of bundle buddy to be reusable in fluidframework. This change is a minimal set of changes needed for us to write a wrapper around it to achieve our goals around package size reporting (in a follow up review, added to a different package). This package will be further refactored after so that bohemia can consume it in a similar way.