-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
feat: allow for specifying path of root turbo.json #9087
feat: allow for specifying path of root turbo.json #9087
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
8 Skipped Deployments
|
let config = sources.into_iter().try_fold( | ||
ConfigurationOptions::default(), | ||
|mut acc, current_source| { | ||
let mut current_source_config = current_source.get_configuration_options(&acc)?; |
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.
There was an existing bug where we would ignore any errors that came from resolving config options from a source. In reality since these were (mostly) eagerly calculated it only affected and validation errors from extracting config options from a turbo.json
.
We now bubble up any errors generated by resolving config options.
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.
Very nicely done! Love the PR organization. Just a couple nits and one question about task access config
if !self.enabled { | ||
return None; | ||
} | ||
let trace_json_path = self.repo_root.join_components(&TASK_ACCESS_CONFIG_PATH); |
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.
In the original code, we first resolve the dir
arg then join to TASK_ACCESS_CONFIG_PATH
. Do we no longer need the dir
part?
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.
We do not need to as traced turbo.json
is only supported for single package repositories so it would only be successful for dir == ""
where it ends up being at $REPO_ROOT/.turbo/traced-config.json
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.
Not required in this PR we can fix this when we need it but this should be aware of the cache dir as well beacuse the intention is that the config gets cached as an artifact.
Co-authored-by: Nicholas Yang <[email protected]>
Description
Allow for setting the root
turbo.json
path via--root-turbo-json=path/to/my.json
orTURBO_ROOT_TURBO_JSON=path/to/my.json
. This option is not compatible with watch mode.Reviewing this PR should be done commit-wise, there's a lot of prefactoring before the final commit that implements the actual feature.
Major changes as part of prefactor:
turbo.json
based on absolute paths instead of repo root relative ones. This allows us to read aturbo.json
from wherever a user wantsturbo.json
outside of the primaryturbo.json
reading logic and intoTaskAccess
config
layering happens instead of ordering them from least to most significant and always choosing a present value, we now go from most to least significant and only choose a value if we do not already have a value for it. This is necessary as we now have a config option (TURBO_ROOT_TURBO_JSON
) that alters a less significant config source (turbo.json
).config.rs
turbo.json
we read after partial evaluation of config sourcesTesting Instructions
Existing unit tests for the refactors.
Manual testing of the traced config logic.
Integration test for new feature.