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: add go struct definitions of OSV record #333

Merged
merged 4 commits into from
Feb 6, 2025

Conversation

michaelkedar
Copy link
Contributor

Moving the struct definitions of the OSV record from osv-scanner to this repository, since it makes more sense to live in the osvschema module.

I've also added the new upstream field to the struct. It's probably worth having a discussion on how to make sure these structs remain in-sync with any future changes to the schema.

Signed-off-by: Michael Kedar <[email protected]>
Copy link
Collaborator

@andrewpollock andrewpollock left a comment

Choose a reason for hiding this comment

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

I personally think this is the right place to consolidate such code, so thanks for creating this PR. Some readability feedback.

michaelkedar and others added 2 commits February 5, 2025 09:31
Co-authored-by: Andrew Pollock <[email protected]>
Signed-off-by: Michael Kedar <[email protected]>
Signed-off-by: Michael Kedar <[email protected]>
Copy link
Collaborator

@another-rex another-rex left a comment

Choose a reason for hiding this comment

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

LGTM. I'm guessing the reason we don't need unmarshal functions is because the default one should just work?

@michaelkedar
Copy link
Contributor Author

LGTM. I'm guessing the reason we don't need unmarshal functions is because the default one should just work?

Yeah, the custom marshal methods are just for omitting empty struct fields and making sure timestamps are in UTC - there's no customized encoding going on.

@michaelkedar
Copy link
Contributor Author

I can't merge this PR myself - I'm happy for this to be merged if you are.

@andrewpollock andrewpollock merged commit a26e1c8 into ossf:main Feb 6, 2025
4 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.

3 participants