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

Add support for metadata component #118

Merged
merged 2 commits into from
Jan 13, 2022

Conversation

artem-smotrakov
Copy link
Contributor

@artem-smotrakov artem-smotrakov commented Jan 4, 2022

Part of #6

@madpah
Copy link
Collaborator

madpah commented Jan 4, 2022

Thanks for the PR @artem-smotrakov. We're in the process of a notable update at present to support the forthcoming CycloneDX 1.4 schema - so please forgive us if we wait for that work to stabilise before we look at merging your PR.

Cheers!

@jkowalleck
Copy link
Member

jkowalleck commented Jan 4, 2022

Thanks for the PR @artem-smotrakov. We're in the process of a notable update at present to support the forthcoming CycloneDX 1.4 schema - so please forgive us if we wait for that work to stabilise before we look at merging your PR.

Cheers!

enabled CI to run CT/QA.

in the meantime: could you check that all tests pass, @artem-smotrakov ?
see https://github.com/CycloneDX/cyclonedx-python-lib/blob/main/CONTRIBUTING.md for more.

@jkowalleck jkowalleck self-requested a review January 4, 2022 13:51
@artem-smotrakov
Copy link
Contributor Author

Hmm, I read the contributing guidelines and ran tests and poetry run autopep8 --in-place -r . before opening the PR. They passed. Let me check again.

@artem-smotrakov
Copy link
Contributor Author

I've fixed several formatting and typing issues. poetry run tox passed. Please re-run the CI/CD jobs.

@jkowalleck jkowalleck added the enhancement New feature or request label Jan 6, 2022
@madpah
Copy link
Collaborator

madpah commented Jan 13, 2022

Hi @artem-smotrakov - we've just merged changes for CycloneDX 1.4 - are you willing to rebase this branch from main?

Thanks, Paul

@artem-smotrakov artem-smotrakov force-pushed the better-metadata branch 2 times, most recently from 5b9f94b to 623ac38 Compare January 13, 2022 12:25
@artem-smotrakov
Copy link
Contributor Author

@madpah I've rebased it -- please rerun CI/CD and have a look at the updates.

I am not sure about this one:

https://github.com/CycloneDX/cyclonedx-python-lib/pull/118/files#diff-9c84a99c5332704dd6a90a4defadfd9df437336f07b79b91033e0a96bbd8fbf1R95

I've noticed that if a BOM doesn't have components, _specialise_output_for_schema_version() fails with a key error. I've updated it to always add components element. That is how XML encoder behaves.

Copy link
Collaborator

@madpah madpah left a comment

Choose a reason for hiding this comment

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

@artem-smotrakov - thanks again for the PR.

It's interesting you have pointed out that components is output as an empty element in some cases - ideally this should not be output if there are no Components - but we are happy to resolve this as a separate bug later.

cyclonedx/model/bom.py Outdated Show resolved Hide resolved
cyclonedx/output/xml.py Outdated Show resolved Hide resolved
@artem-smotrakov
Copy link
Contributor Author

@madpah I've addressed your comments -- thanks for the review!

@artem-smotrakov artem-smotrakov force-pushed the better-metadata branch 2 times, most recently from 9f71e27 to 551813f Compare January 13, 2022 13:44
artem-smotrakov and others added 2 commits January 13, 2022 13:46
Part of CycloneDX#6

Signed-off-by: Artem Smotrakov <asmotrakov@riotgames.com>
Signed-off-by: Artem Smotrakov <asmotrakov@riotgames.com>
@madpah madpah merged commit 1ac31f4 into CycloneDX:main Jan 13, 2022
@artem-smotrakov artem-smotrakov deleted the better-metadata branch January 13, 2022 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants