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 support for Lifecycles in BOM metadata #698

Merged
merged 14 commits into from
Oct 21, 2024
Merged

feat: add support for Lifecycles in BOM metadata #698

merged 14 commits into from
Oct 21, 2024

Conversation

Churro
Copy link
Contributor

@Churro Churro commented Oct 9, 2024 •
edited by jkowalleck
Loading

part of #578

@Churro Churro requested a review from a team as a code owner October 9, 2024 22:40
@jkowalleck jkowalleck added enhancement New feature or request schema 1.5 labels Oct 10, 2024
jkowalleck
jkowalleck requested changes Oct 10, 2024
View reviewed changes
Copy link
Member

@jkowalleck jkowalleck left a comment •
edited
Loading

Choose a reason for hiding this comment

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

The implementation is really well thought and has almost every small detail perfectly correct. 🥇

There are just a very small amount of things that needs changing, marked with ❌ .

In addition, please add the needed enum tests to tests/test_enums.py.

@jkowalleck jkowalleck mentioned this pull request Oct 7, 2024
Open
15 tasks
@Churro
review feedback
8cff89e 
Signed-off-by: Johannes Feichtner <[email protected]>
@Churro
Copy link
Contributor Author

Churro commented Oct 10, 2024

I tried to align the class names as closely as possible with the CDX 1.5 specification to avoid potentially confusing homonyms.

I've also seen that cyclonedx-javascript-library uses LifecyclePhase, NamedLifecycle and cyclone-core-java uses LifecycleChoice. Related to these, would the following naming suggestion be ok for you?

  • Phase -> LifecyclePhase
  • CustomPhase -> NamedLifecycle
  • PredefinedPhase -> PredefinedLifecycle

@jkowalleck
Copy link
Member

I tried to align the class names as closely as possible with the CDX 1.5 specification to avoid potentially confusing homonyms.

I've also seen that cyclonedx-javascript-library uses LifecyclePhase, NamedLifecycle and cyclone-core-java uses LifecycleChoice. Related to these, would the following naming suggestion be ok for you?

* `Phase` -> `LifecyclePhase`

* `CustomPhase` -> `NamedLifecycle`

* `PredefinedPhase` -> `PredefinedLifecycle`

The renaming you proposed sounds reasonable. Please continue with that.

@jkowalleck jkowalleck self-requested a review October 10, 2024 18:14
@Churro
rename classes
a4deeaf 
Signed-off-by: Johannes Feichtner <[email protected]>
@jkowalleck jkowalleck mentioned this pull request Oct 10, 2024
Merged
@jkowalleck
Copy link
Member

jkowalleck commented Oct 10, 2024 •
edited
Loading

@Churro,

I've proposed some changes on top of yours: Churro#1
If you would simply merge my pull request, your pullrequest would update automatically.

PS: was done via df0fdf0

jkowalleck and others added 6 commits October 11, 2024 01:45
@jkowalleck
fix repr
2cf3dd8 
Signed-off-by: Jan Kowalleck <[email protected]>
@jkowalleck
typos and docs
17f3445 
Signed-off-by: Jan Kowalleck <[email protected]>
@jkowalleck
fix setter
52f6579 
Signed-off-by: Jan Kowalleck <[email protected]>
@jkowalleck
condense code
0a44190 
Signed-off-by: Jan Kowalleck <[email protected]>
@jkowalleck
fix test
0e842ad 
Signed-off-by: Jan Kowalleck <[email protected]>
@Churro
Merge pull request #1 from CycloneDX/Churro/feat/lifecycles
df0fdf0 
changes on top of PR#698
@jkowalleck
Copy link
Member

jkowalleck commented Oct 11, 2024 •
edited
Loading

@madpah could you review the current state of this PR? It aims to cover $.metadata.lifecycles from #578

@jkowalleck jkowalleck requested a review from a team October 11, 2024 11:20
@jkowalleck jkowalleck changed the title feat: add support for lifecycles in BOM metadata feat: add support for Lifecycles in BOM metadata Oct 15, 2024
@jkowalleck
Copy link
Member

@Churro,
Could you rebase onto the latest main branch and fix the conflicts accordingly?

Churro and others added 2 commits October 15, 2024 21:36
@Churro
Merge branch 'main' into feat/lifecycles
86ed153 
Signed-off-by: Johannes Feichtner <[email protected]>
@Churro
fix tests for v8.0.0
bdbb93e 
Signed-off-by: Johannes Feichtner <[email protected]>
@jkowalleck
Copy link
Member

jkowalleck commented Oct 16, 2024 •
edited
Loading

some of the tests are failing.

@Churro , you may run them locally as described in https://github.com/CycloneDX/cyclonedx-python-lib/blob/main/CONTRIBUTING.md

PS: done so via bc74a86

@Churro
fix issues after merge
bc74a86 
Signed-off-by: Johannes Feichtner <[email protected]>
@jkowalleck jkowalleck self-requested a review October 18, 2024 07:40
@jkowalleck jkowalleck requested review from madpah and a team October 18, 2024 07:46
@jkowalleck jkowalleck merged commit 6cfeb71 into CycloneDX:main Oct 21, 2024
41 checks passed
@jkowalleck
Copy link
Member

thank you for contributing this feature, @Churro .

it was released via v8.1.0 🎉

saquibsaifee pushed a commit to saquibsaifee/cyclonedx-python-lib that referenced this pull request Oct 27, 2024
@Churro @jkowalleck @saquibsaifee
feat: add support for Lifecycles in BOM metadata (CycloneDX#698)
9a3a45e 
---------

Signed-off-by: Johannes Feichtner <[email protected]>
Signed-off-by: Jan Kowalleck <[email protected]>
Signed-off-by: Johannes Feichtner <[email protected]>
Co-authored-by: Jan Kowalleck <[email protected]>
Signed-off-by: Saquib Saifee <[email protected]>
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

@madpah madpah Awaiting requested review from madpah

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

Successfully merging this pull request may close these issues.
2 participants
@Churro @jkowalleck