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
fix: add missing Vulnerability.properties
types in schema 1.4
#148
Conversation
Signed-off-by: Mike de Senna <desenna@gmail.com>
Signed-off-by: Mike de Senna <desenna@gmail.com>
Signed-off-by: Mike de Senna <desenna@gmail.com>
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.
see my annotations.
please have them sorted out
@jkowalleck Thanks for the review. I responded to the comments with explanations, but don't understand what's wrong with them. Please feel free to elaborate on what specifically I'm getting wrong and I'll be happy to fix them. Also, if you know how to develop / test locally, can you please share that info with me so I can do? It's possible / likely that I made a mistake here somewhere, but I didn't see how to run tests in the instructions and afaik CI hasn't kicked off fully here so I can't see what steps are taken to setup and run tests. Thanks again! |
@stevespringett can kick off the CI. |
@jkowalleck @stevespringett I'm not able to understand what is causing the test to fail. I can run it locally but I don't see anything except that false is not true. Do you have any advice on how to proceed with this? |
@desenna don't worry about the failure. The CycloneDX Core Java library is the thing that tests it, and it uses local schemas, not the updates you've submitted. |
Thanks @stevespringett makes sense. So are there any other steps I should take here? Or is it ready to merge and deploy you think? Asking only because I can then get to work on resolving CycloneDX/cyclonedx-go#40 Thanks and have a great weekend! |
@desenna Although this is a defect, it will have a negative impact on everyone using v1.4 at the moment. In order for this to move forward, the core working group would need to vote on incorporating this into v1.4.2 of the XML schema, or wait until v1.5 next year. The next Core Working Group meeting is on Monday June 6. I'll bring this topic up then. |
SGTM! I agree this is a good idea to bring up during Core Working Group meeting and that a v1.4.2 patch version makes sense here. Another option might even be to leave it as v1.4.1 (if that's where it's currently at) since this doesn't represent a breaking or backwards-incompatible change but instead completes the specification for XML and protobuf, whereas incrementing a patch version to v1.4.2 in my mind is more like a fix of something that was incorrect. The semantics are seemingly minor, but seems nothing was wrong here.. just incomplete by one field. This carries the side benefit of also giving other library maintainers - like cyclonedx-go and others - the room/time to make changes in those libraries as well, which their consumers would be able to leverage or even make the open source contributions themselves! |
Ok this will be merged in later this week, but not released yet. We will be updating all official libraries with the updated schema and releasing all updates around the same time. |
HI @stevespringett just to confirm is this the fix for java? CycloneDX/cyclonedx-core-java#208 |
Confirmed. Had vacation, then COVID, but back at it now. Will merge in the next few days. |
Closes #147