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.

Sign up for GitHub

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

Jump to bottom

feat: add factory method XsUri.make_bom_link() #728

Merged
merged 14 commits into from
Oct 29, 2024
Merged

feat: add factory method XsUri.make_bom_link() #728

merged 14 commits into from
Oct 29, 2024

Conversation

saquibsaifee
Copy link
Contributor

@saquibsaifee saquibsaifee commented Oct 26, 2024 •
edited
Loading

fixes #726

@saquibsaifee saquibsaifee mentioned this pull request Oct 26, 2024
Closed
Saquib Saifee added 2 commits October 26, 2024 21:24
jkowalleck
jkowalleck reviewed Oct 27, 2024
View reviewed changes
Copy link
Member

@jkowalleck jkowalleck left a comment

Choose a reason for hiding this comment

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

👍 looks promising.
please add unit test.

cyclonedx/model/bom.py Outdated Show resolved Hide resolved
@saquibsaifee saquibsaifee changed the title [WIP] feat: add helper method to generate bom_link feat: add helper method to generate and get bom_link Oct 27, 2024
@saquibsaifee saquibsaifee marked this pull request as ready for review October 27, 2024 17:07
@saquibsaifee saquibsaifee requested a review from a team as a code owner October 27, 2024 17:07
@saquibsaifee
Copy link
Contributor Author

saquibsaifee commented Oct 27, 2024 •
edited
Loading

@jkowalleck is ready for review.

It would be great if we could validate the existence of bom_ref in the Bom() inside the get_bom_link. We can leverage at least get_vulnerabilities_for_bom_ref, but unsure how to validate in other places.

jkowalleck
jkowalleck requested changes Oct 28, 2024
View reviewed changes
cyclonedx/model/bom.py Outdated Show resolved Hide resolved
tests/test_model.py Outdated Show resolved Hide resolved
tests/test_model.py Outdated Show resolved Hide resolved
tests/test_model.py Outdated Show resolved Hide resolved
tests/test_model.py Outdated Show resolved Hide resolved
@jkowalleck jkowalleck added the enhancement New feature or request label Oct 28, 2024
@jkowalleck jkowalleck changed the title feat: add helper method to generate and get bom_link feat: add factory method XsUri.make_bom_link() Oct 28, 2024
perf: utilize urllib for encoding
42fe470 
Signed-off-by: Saquib Saifee <[email protected]>
jkowalleck
jkowalleck requested changes Oct 28, 2024
View reviewed changes
cyclonedx/model/__init__.py Outdated Show resolved Hide resolved
cyclonedx/model/__init__.py Outdated Show resolved Hide resolved
cyclonedx/model/__init__.py Outdated Show resolved Hide resolved
cyclonedx/model/__init__.py Show resolved Hide resolved
Saquib Saifee added 2 commits October 28, 2024 12:48
jkowalleck
jkowalleck reviewed Oct 28, 2024
View reviewed changes
Copy link
Member

@jkowalleck jkowalleck left a comment

Choose a reason for hiding this comment

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

all requirements done, i have an idea for refactoring (see below).
what do you think, does this make sense?

cyclonedx/model/__init__.py Outdated Show resolved Hide resolved
@jkowalleck jkowalleck self-requested a review October 28, 2024 17:42
@jkowalleck jkowalleck requested a review from a team October 28, 2024 18:09
@jkowalleck jkowalleck merged commit 5ec73d0 into CycloneDX:main Oct 29, 2024
47 checks passed
@jkowalleck
Copy link
Member

was released via https://github.com/CycloneDX/cyclonedx-python-lib/releases/tag/v8.4.0

@saquibsaifee saquibsaifee deleted the add-bom-link-helper branch November 12, 2024 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jkowalleck jkowalleck jkowalleck approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: add helper method to generateBomLink
2 participants
@saquibsaifee @jkowalleck