-
-
Notifications
You must be signed in to change notification settings - Fork 41
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 cyclonedx.model.dependency.Dependency.provides
#735
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Uzair Chhapra <[email protected]>
f2ad0ed
to
fb4598d
Compare
Signed-off-by: Uzair Chhapra <[email protected]>
@jkowalleck PR is ready for review. |
cyclonedx.model.dependency.Dependency.provides
cyclonedx.model.dependency.Dependency.provides
thank you for your contribution, @uzairchhapra . the implementation looks promising. Regarding tests, we tend to go with an integration-test snapshot-solution, over detailed unit tests. |
Signed-off-by: Uzair Chhapra <[email protected]>
Signed-off-by: Uzair Chhapra <[email protected]>
Signed-off-by: Uzair Chhapra <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
self, | ||
target: Dependable, | ||
depends_on: Optional[Iterable[Dependable]] = None, | ||
provides: Optional[Iterable[Dependable]] = None, |
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.
instead of adding a new parameter here, how about adding a new method instead: register_provision(self, target: Dependable, provides: Optional[Iterable[Dependable]] = None)
.
what do you think about this?
this would fit the original architectural plans better.
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'll give this a try
@@ -0,0 +1,113 @@ | |||
{ |
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.
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.
this could be fixed by a view filter.
here is an example:
cyclonedx-python-lib/cyclonedx/model/bom.py
Lines 193 to 200 in 5788bc1
@property | |
@serializable.view(SchemaVersion1Dot2) | |
@serializable.view(SchemaVersion1Dot3) | |
@serializable.view(SchemaVersion1Dot4) | |
@serializable.view(SchemaVersion1Dot5) | |
@serializable.view(SchemaVersion1Dot6) | |
@serializable.xml_sequence(6) | |
def manufacture(self) -> Optional[OrganizationalEntity]: |
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 tried adding it here:
@serializable.view(SchemaVersion1Dot6) |
The tests still seem to fail for all other SBOM specs (v1.0 to v1.5).
I am sure I must be missing something here...
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.
ran all tests, all passed.
see https://github.com/CycloneDX/cyclonedx-python-lib/actions/runs/11883414982/job/33123440693?pr=735
Signed-off-by: Uzair Chhapra <[email protected]>
@@ -1310,6 +1310,28 @@ def get_bom_with_definitions_standards() -> Bom: | |||
) | |||
|
|||
|
|||
def get_bom_v1_6_with_provides() -> Bom: |
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.
please rename to get_bom_with_provides
.
there is no intention to have models for certain CDX versions only.
In fact, it is intended to test the serialization with a target that is expected to omit certain parts.
Fixes #691