-
Notifications
You must be signed in to change notification settings - Fork 40
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
[2/6] [oxql] move core types to a new oxql-types crate #6364
[2/6] [oxql] move core types to a new oxql-types crate #6364
Conversation
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
Created using spr 1.3.6-beta.1 [skip ci]
Created using spr 1.3.6-beta.1
Means that the Nexus external API crate doesn't have to pull in oximeter-db. Pull Request: oxidecomputer#6364
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.
Wow, this one looks non-trivial, thanks for digging into it! I have one request, that we move TimeseriesKey
around a bit, since it's more than an OxQL concept. Other than that, LGTM!
Created using spr 1.3.6-beta.1
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.
Looks good, thanks for moving over the TimeseriesKey
type!
At long last, here's the last OpenAPI document converted into a trait: the Nexus external API. This is a fairly large change, but with all the prerequisites in place the conversion is overall pretty mechanical. I did need to move some types around, but the bulk of the PR is just adding the very large external API trait. This PR also adds a `.git-blame-ignore-revs` entry corresponding to #6372, so that the indent doesn't ruin blame in the GitHub web view. (You can configure this locally as well with `git config blame.ignoreRevsFile .git-blame-ignore-revs`.) In a subsequent PR I'll remove the old instructions. Depends on: * #6355 * #6364 * #6369 * #6370 * #6372 Co-authored-by: David Crespo <[email protected]>
Means that the Nexus external API crate doesn't have to pull in oximeter-db.
I did have to expose a few mutators, but my general reasoning is that because these types derive
Deserialize
, it's always possible to make these changes by serializing into JSON and working with them there.Depends on #6355.