-
Notifications
You must be signed in to change notification settings - Fork 274
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
perf: improve preprocess action perf by capturing context instead of partially resolving template strings #6745
base: main
Are you sure you want to change the base?
Conversation
partially resolving template strings
...config, | ||
...resolvedBuiltin, | ||
spec, | ||
} |
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.
TODO: can we somehow keep early validation? Is it dangerous to remove?
const resolvedDeps = capture( | ||
rawConfig.build.dependencies as unknown as ParsedTemplate, | ||
configContext | ||
) as unknown as BuildDependencyConfig | ||
|
||
// The build.dependencies field may not resolve at all, in which case we can't extract any deps from there | ||
if (isArray(resolvedDeps)) { |
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.
todo: this branch needs adaptation, as we do not resolve dependencies anymore.
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.
do dependencies need to be fully resolved? If partial is ok, is it then fine not to resolve them at all?
The yaml highlighting did not work in production builds anyway and removing this functionality made it easier to compare performance between production builds and the dev build for commands like `get graph`.
99f72e9
to
cb0081c
Compare
fail early if we access an unresolved template value
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer: