-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core(user-flow): audit flow from artifacts json #13715
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I expected this to work with -GA
, but I guess that will come later? If so, please rename the PR title to reflect that this isn't actually loading or saving artifacts to disk during normal operation.
const configJson = gatherStep.config || flowConfig; | ||
const {gatherMode} = artifacts.GatherContext; | ||
const {config} = initializeConfig(configJson, {...configContext, gatherMode}); | ||
runnerOptions = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seems we could add a second property like _gatherStepRunnerOptions
or something, and keep runner options data there. could be an array or a weakmap (if array, this loop changes to for (let i ...
, if weakmap can keep for loop as is). I think conceptually the weakmap makes sense here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I decided to go with the weak map approach here. Keeps GatherStep
type clear of any extra stuff
Steps 4 & 5 of #13364. I combined them because I needed step 5 to test step 4 🤷
Lol +900,000 from the artifacts file. It contains the DT logs and traces inline for the equivalent of 3 normal LH runs.
Ref
#11313