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

feat: variants in metadata #57

Merged
merged 2 commits into from
Jan 16, 2025

Conversation

baszalmstra
Copy link
Contributor

Refactors the code to use passed in variants for both build and metadata.

@baszalmstra baszalmstra requested a review from tdejager January 13, 2025 16:15
Copy link
Contributor

@tdejager tdejager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is a big amount of duplication, can you maybe add a card for it on the build board. Other than that its a few docstrings.

@@ -362,6 +348,38 @@ impl CMakeBuildBackend {
sandbox_config: None,
})
}

pub fn compute_variants(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could use some docs

@@ -322,31 +323,15 @@ impl PythonBuildBackend {
}

/// Returns the build configuration for a recipe
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets improve these docs to be a bit less general :)

@@ -397,6 +381,38 @@ impl PythonBuildBackend {
sandbox_config: None,
})
}

pub fn compute_variants(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs

@baszalmstra
Copy link
Contributor Author

I added more docs to the PR, let me know what you think! @tdejager

@tdejager tdejager merged commit 377e95d into prefix-dev:main Jan 16, 2025
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Variants should work for all backend not only the recipe backend.
2 participants