Conversation
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
crates/spk-schema/src/spec.rs
Outdated
|
|
||
| pub fn from_yaml<S: Into<String>>(yaml: S) -> Result<SpecFileData> { | ||
| /// Parse the provided string as a yaml-encoded [`SpecFileData`]. | ||
| pub fn from_yaml<S: Into<String>>(yaml: S) -> Result<Self> { |
There was a problem hiding this comment.
I've being meaning to ask if the decision to use Into<String> instead of AsRef<str> (or Borrow?) was out of necessity. From what I've seen the argument doesn't need to be an owned value, and there are potentially callers that could pass a reference in here and avoid allocating a copy.
There was a problem hiding this comment.
Ya, IIRC the yaml error needs an owned string and the only use case for this was call sites (on the cli) that never needed to retain ownership of the original string. I figured it was better to avoid cloning on the error cases since there was no good reason not to... I'm just as happy if we want it the other way though, API ergonomics over optimizing that case...
crates/spk-schema/src/template.rs
Outdated
| return Err(serde::de::Error::missing_field("pkg or var")); | ||
| }; | ||
| match first_key.as_ref() { | ||
| "gitTags" => Ok(DiscoverStrategy::GitTags(Deserialize::deserialize( |
There was a problem hiding this comment.
Can you explain the peekable map stuff here? It looks like this is just manually implementing what you get from a tagged enum. But the struct is marked as untagged.
There was a problem hiding this comment.
Scratch what I said, I see that it is not like the available tagged enum flavors. However I do still have a concern that this requires that the first key is "gitTags" when it would otherwise be perfectly legal to have the content in the yaml file in some other order. For example, maybe a yaml prettier decides it wants to reorder the mapping and "gitTags" isn't first anymore.
You'd need to peek the whole mapping and check if it contains that key.
There was a problem hiding this comment.
The internally tagged flavor isn't terrible or so different from what we're trying to do and then we could just derive the code.
There was a problem hiding this comment.
Ya, we need to avoid the generic error message from untagged enum because so much of the complexity is within the parsing of these items...
The peekable is my attempt to create a more elegant way to pick a variant based on the presence of a field, we actually had this discussion a long time ago I think re introducing the field order requirement, but I can't say with certainty that we settled it.
The internally tagged is okay, but strays from our existing convention and the design that we settled on in discussions, namely: #1121 (comment)
something like this?
api: v1/platform
platform: vfx-reference/2024
requirements:
- kind: package
name: python
build:
version: "3.10"There was a problem hiding this comment.
From the meeting today, don't require field order - stick to existing patterns
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
This currently cannot return data in many cases but will evolve as other areas of the workspace setup do, too. Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
This commit augments the templating system for spk recipes,
enabling the definition of multiple package versions from a single
recipe file.
Key changes include:
- **`spk-schema`**:
- Updated `Template` trait and `SpecTemplate` struct to represent
recipe templates.
- A `v1::Platform` spec for defining platforms in a more structured
way.
- The `SpecRecipe` enum now includes `V1Platform`.
- **`spk-workspace`**:
- The `Workspace` and `WorkspaceBuilder` are updated to work with
the new `SpecTemplate` and templating system. Specifially,
workspace items can no longer specify versions - this is done
directly in the spec file now.
- **`spk-cli`**:
- The `make-recipe`, `workspace info`, and `view` commands are
updated to support the new templating system.
- **`spk-storage`**:
- The `WorkspaceRepository` is updated to work with the new
templating system, presenting versions based on the template.
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com>
Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Adds a serde-peekable crate with a simple wrapper that allows peeking map keys so that custom deserialize methods can appropriately predict enum variants Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
This allows us to enalbe source builds for requests that define an explicit repo, provided that it has a source package for the package and version. Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Because templates may support many versions of a package, the existing method of turning ranges into LowestSpecified can make it so that there's no way for users to disambiguate the actual version that they want. By supporting version ranges, the = and == syntax can be used. Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
The presence of this field enables build-from-source for these packages within a workspace, and allows us to expand on the configuration for these builds in the future. Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <rbottriell@ilm.com> Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
Signed-off-by: Ryan Bottriell <ryan@bottriell.ca>
2 participants
Opening this early just so that I can link to some of the changes and docs...