-
Notifications
You must be signed in to change notification settings - Fork 18
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
lib/osv: more schema alignment improvements, and other cleanups #64
lib/osv: more schema alignment improvements, and other cleanups #64
Conversation
A general-purpose OSV library should not hardcode anything about the `database_specific` or `ecosystem_specific` fields. Abstract over these. A naïve consumer can parse `Model Value Value Value Value` for no loss of information. A producer can instantiate unused database/ecosystem-specific fields at `Void`. `()` is not recommended, because `Just ()` will serialise as an empty JSON array.
We might use something like the `Affects` data type the HSEC `database_specific` or Hackage `ecosystem_specific` fields. But this does not belong in the general-purpose OSV module. So remove it. (It might get reincarnated elsewhere, in a later commit).
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.
I think that abstracting is a bit early though, adding a bit too much complexity.
The question is - do we want a general purpose library or something specific to our use case? From my POV it would be a shame to implement all the OSV machinery but hardcode the db/ecosystem-specific fields to our use case. I don't think it adds much complexity for users - at worst a few type annotations to declare the payload types. |
I'm also leaning in favor of a general purpose library if we're already working on it. |
Another approach would be But I see it got merged. Thanks for the reviews :) FWIW I do intend to extract |
Note: 4 self-contained, sequential commits. Recommend to review each in sequence.