Skip to content
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

chore(config): leverage proc macros #9111

Merged
merged 4 commits into from
Sep 10, 2024

Conversation

chris-olszewski
Copy link
Member

@chris-olszewski chris-olszewski commented Sep 5, 2024

Description

This PR leverages derive_setters and merge to reduce the amount of changes required when adding a field to the configuration options.

The manual setup is error prone as there are 4 places that need to be updated and forgetting the final place will result in code compiling, but the field never getting layered properly.

Testing Instructions

Existing unit & integration tests, but also looking what the proc macros are generating (outputs shown are produced using rust-analyzer's Expand macro feature:

Output of #[derive(Setters)]

impl TurborepoConfigBuilder {
    pub fn with_api_url(mut self, value: Option<String>) -> Self {
        self.override_config.api_url = value;
        self
    }
...

Which exactly matches what create_builder would produce:

pub fn with_api_url(mut self, value: Option<String>) -> Self {
    self.override_config.api_url = value;
    self
}

#derive(Merge) produces

impl ::merge::Merge for ConfigurationOptions {
    fn merge(&mut self, other: Self) {
        ::merge::Merge::merge(&mut self.api_url, other.api_url);
...

and Merge is defined for Option as :

impl<T> Merge for Option<T> {
    fn merge(&mut self, mut other: Self) {
        if !self.is_some() {
            *self = other.take();
        }
    }
}

source

So self.api_url will only be set to other.api_url if there isn't a value present which is the behavior we want since we merge configs from highest to lowest precedence.

Copy link

vercel bot commented Sep 5, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
examples-nonmonorepo ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 10, 2024 2:51pm
8 Skipped Deployments
Name Status Preview Comments Updated (UTC)
examples-basic-web ⬜️ Ignored (Inspect) Visit Preview Sep 10, 2024 2:51pm
examples-designsystem-docs ⬜️ Ignored (Inspect) Visit Preview Sep 10, 2024 2:51pm
examples-gatsby-web ⬜️ Ignored (Inspect) Visit Preview Sep 10, 2024 2:51pm
examples-kitchensink-blog ⬜️ Ignored (Inspect) Visit Preview Sep 10, 2024 2:51pm
examples-native-web ⬜️ Ignored (Inspect) Visit Preview Sep 10, 2024 2:51pm
examples-svelte-web ⬜️ Ignored (Inspect) Visit Preview Sep 10, 2024 2:51pm
examples-tailwind-web ⬜️ Ignored (Inspect) Visit Preview Sep 10, 2024 2:51pm
examples-vite-web ⬜️ Ignored (Inspect) Visit Preview Sep 10, 2024 2:51pm

crates/turborepo-lib/src/config/mod.rs Outdated Show resolved Hide resolved
Co-authored-by: Nicholas Yang <nicholas.yang@vercel.com>
@chris-olszewski chris-olszewski merged commit bd2bffa into main Sep 10, 2024
40 checks passed
@chris-olszewski chris-olszewski deleted the olszewski/chore_proc_macro_config branch September 10, 2024 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants