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

Abstract SPDX versioning #175

Closed
ivanayov opened this issue Dec 5, 2022 · 4 comments · Fixed by #172
Closed

Abstract SPDX versioning #175

ivanayov opened this issue Dec 5, 2022 · 4 comments · Fixed by #172

Comments

@ivanayov
Copy link

ivanayov commented Dec 5, 2022

Hi,

Currently most types and methods are named after the SPDX version they parse/represent, which is reasonable for the parser itself, but makes adoption much harder.
It would be great if tools-golang provides some abstraction, so that any tools using it can update the SPDX version via configuration only, rather than doing code updates or refactoring every time, as those are necessary in the parser only.

@lumjjb
Copy link
Collaborator

lumjjb commented Dec 14, 2022

HI @ivanayov! Thanks for opening up this issue! Yes, the abstraction need some work and we have an open issue to discuss it (#152)... @kzantow is currently working on the reflection code implementation to provide that abstraction! We'd be happy to get more feedback on how you'd like to use it though!

@kzantow
Copy link
Collaborator

kzantow commented Dec 15, 2022

Hi @ivanayov -- yes, I currently have a draft PR (#172) that does this. I upgraded Syft to use 2.3 and it was a lot more difficult than it needed to be, so I suggested some refactoring that would make downstream tooling easier. I'm going to try to get this PR to the finish line in the next few days, which won't make your immediate upgrade easier, but should make all future upgrades significantly easier. I'm also going to make a branch in Syft that uses this new model to make sure it works as expected.

@kzantow
Copy link
Collaborator

kzantow commented Dec 16, 2022

@ivanayov I've written up a bit of an explanation of the changes I'm hoping to get merged -- do they make sense to you and seem to provide the functionality you're looking for? See the description here: #172 (comment)

@ivanayov
Copy link
Author

ivanayov commented Jan 6, 2023

Thank you @lumjjb and @kzantow! I moved the discussion to #172.

Closing as it partially duplicates #152, but I think #172, which is not strictly following the #152 proposal, covers it well and is closer to this issue. I left my comments in the PR.

@ivanayov ivanayov closed this as completed Jan 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants