remove the SemverVersion wrapper struct
#7570
Conversation
iliana
force-pushed
the
iliana/semver
branch
from
feb259e to
2f1f0d7
Compare
| { | ||
| type = "string", | ||
| pattern = r"^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$", |
There was a problem hiding this comment.
fascinating, til!
Is there a worry that people will forget to do it?
There was a problem hiding this comment.
I have several concerns about this approach but it seems least bad given that schemars decided semver::Version ought to be treated as a literal :(
If you don't add this, Progenitor generates a wrapper struct around a String. I don't think it matters that much if you leave it as-is; either you use that, or you get annoyed enough with it that you realize you can do this.
There was a problem hiding this comment.
Oh actually, does schemars come with an impl for semver::Version? If so then maybe specifying the pattern again isn't required? not sure.
There was a problem hiding this comment.
Even moving the pattern into a const and using it here doesn't work; the macro wants a literal. (I think it's being directly deserialized into the relevant schemars struct but I didn't get that far into figuring out the Progenitor source.)
There was a problem hiding this comment.
Oh I bet that's because it accepts a string and not a ParseWrapper<syn::Expr>.
There was a problem hiding this comment.
Ah looks like it actually needs the SchemaObject to compare at compile time against. https://github.com/oxidecomputer/progenitor/blob/4a8467a4660df4308890b690808ecb898f128cd1/progenitor-macro/src/lib.rs#L157
Seems unavoidable in that case, I think.
What is TODO'd shall be todone:
omicron/common/src/api/external/mod.rs
Lines 491 to 492 in b9cc035
We'd like to split tufaceous out into its own repo and and this wrapper struct makes it a little difficult; may as well clean this up in the process.