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: extends manifest to allow for preview features #2489

Merged
merged 6 commits into from
Nov 18, 2024

Conversation

tdejager
Copy link
Contributor

@tdejager tdejager commented Nov 15, 2024

Description

This PR adds the ability to allow the use of preview features, we want to use this to enable certain features that should be enabled on the manifest level. The first one being, the workspace and build functionality. The idea behind these features is that we can still break and test out new workflows while being in preview, but we do like to get some hands-on testing for these features.

Say for example we allow for a new solver (don't worry we are not really doing this :)), you could then specify:

[project]
# ... more metadata
preview = ["new_solver"]

Alternatively, to allow for all preview features one can use preview = true.

Notes

I've modified the schema and the documentation for this change. No features have been specified as of yet, but it should be relatively straightforward by adding an enum value to the KnownPreviewFeatures enum.

Testing

I think most of the testing for this is covered in CI for now. Best to look at the API in project and see if this is something that looks good for now.

@tdejager tdejager changed the title wip: preview feature feat: extends manifest to allow for preview features Nov 18, 2024
@tdejager tdejager marked this pull request as ready for review November 18, 2024 11:00
schema/model.py Show resolved Hide resolved
src/project/mod.rs Outdated Show resolved Hide resolved
Copy link
Contributor

@nichmor nichmor left a comment

Choose a reason for hiding this comment

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

I like it

crates/pixi_manifest/src/validation.rs Show resolved Hide resolved
Copy link
Contributor

@ruben-arts ruben-arts left a comment

Choose a reason for hiding this comment

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

Looks good, a bit hard to test as there is no preview feature yet. But works as expected so far.

I do think it's good to use serde_ignored for the warning to streamline that workflow over pixi's parsing.

crates/pixi_manifest/src/validation.rs Show resolved Hide resolved
@tdejager
Copy link
Contributor Author

Also checked out if I could do with just an #[serde(untagged)] in the enum list, however, this messes up the error messages again, and flattening KnownPreviewFeature and PreviewFeature had a bit of bad DX when adding and removing from the enum which we would probably do quite a bit.

Now you can just add/remove to the enum and everything just works. So would rather keep like this.

@ruben-arts ruben-arts enabled auto-merge (squash) November 18, 2024 14:51
@ruben-arts ruben-arts merged commit a884600 into prefix-dev:main Nov 18, 2024
44 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.

4 participants