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

📣 v2 finalization #158

Closed
4 of 5 tasks
jkowalleck opened this issue Oct 10, 2022 · 12 comments · Fixed by #266
Closed
4 of 5 tasks

📣 v2 finalization #158

jkowalleck opened this issue Oct 10, 2022 · 12 comments · Fixed by #266
Assignees
Milestone

Comments

@jkowalleck
Copy link
Member

jkowalleck commented Oct 10, 2022

@jkowalleck jkowalleck added this to the v2 milestone Oct 10, 2022
@jkowalleck jkowalleck pinned this issue Oct 26, 2022
@jkowalleck jkowalleck self-assigned this Mar 6, 2023
@jkowalleck
Copy link
Member Author

I know you were waiting for this for a long time, @llaville :D
I will publish a stable RC this weekend. Then you could test it and see it if is any worth .

@jkowalleck jkowalleck changed the title v2 finalization 📣 v2 finalization Mar 11, 2023
@llaville
Copy link

I know you were waiting for this for a long time, @llaville :D I will publish a stable RC this weekend. Then you could test it and see it if is any worth .

Yes, and of course I'll test it, but I'm not sure to have time this week-end.
I'll gave you my feedback at least on monday or later.

@jkowalleck
Copy link
Member Author

jkowalleck commented Mar 11, 2023

released preview: v2.0.0-RC1

@llaville
Copy link

released preview: v2.0.0-RC1

👁️‍🗨️ I'll have a quick try with migration on my project tomorrow morning !

@jkowalleck
Copy link
Member Author

jkowalleck commented Mar 11, 2023

❤️
Hope you have static code analysis in place, to helps you find positions where you need to change your code. Or you have good test coverage.
Otherwise, the API change log is pretty detailed. Let me know if I forgot to document something.

@llaville
Copy link

@jkowalleck I've just finished to migrate code from v1 to v2, and here are my first feedbacks.

Even, if my code (see my migration with commit llaville/box-manifest@e4a4058) is not so complex (I hope ;-)), it took me half an hour between change code from old format to newer, and test it to see if there are some regressions.

I think you should write a migration guide that will make the developers task more easy !

If you've time to check my commit and tell me if I'm wrong or not in my code migration.

I've tested spec 1.3 because I've a base results to compare, but spec 1.4 is pretty for me now to implement on my application (but I've no base to compare), so no feeback for this results.

@llaville
Copy link

llaville commented Mar 12, 2023

@jkowalleck I've just tested spec 1.4 in my application (code of box-manifest) will be pushed to GitHub tomorrow (I'll be busy this afternoon and won't be online for hours), and see no regression with spec 1.3 ! Good news :)

Last but not least: results I produced (using spec 1.3 or spec 1.4) can be validate with https://github.com/CycloneDX/cyclonedx-cli#validate-command

@jkowalleck
Copy link
Member Author

Thanks for testing.

I think you should write a migration guide that will make the developers task more easy !

Well, I might write a migration instruction for v2.0, then.
What kind of document could have helped you? Did you read the detailed change log that was mentioned in the release notes?
I see you have phpstan as a static code analysis. This should have found all breaking code changes. Breaking behavior is described in the mentioned change log, right?

[...] results I produced (using spec 1.3 or spec 1.4) can be validate [...]

I had no doubt on the quality of my library, as I have more than 3400 tests to assert the correctness of the models and normalization/serialization, and I have a working validator as part of the implementation as well ;-)

@llaville
Copy link

Well, I might write a migration instruction for v2.0, then. What kind of document could have helped you? Did you read the detailed change log that was mentioned in the release notes?

Mea Culpa, I didn't read the release notes, and I think it's enough. Perharps you may add a link on the project README page to learn more about how to migrate (to this release notes).

BTW, thanks for your review on my code. I appreciate a lot !!!

@llaville
Copy link

@jkowalleck By implementing support of all sbom spec version with my commit llaville/box-manifest@0ac39af on box-manifest app, I've used twice the tip provided at https://www.php.net/manual/en/language.enumerations.static-methods.php#126866

Does it make sense for you to implement such new method values in your enum Version ?

That will allow me to reduce my code :

Goal : support as much as possible future spec version without to change the base code

@jkowalleck
Copy link
Member Author

jkowalleck commented Mar 12, 2023

Does it make sense for you to implement such new method values in you

I do not see a reason for this.
If native PHP has no method for this, then it is probably for a good reason.
I like the way you solved it via array_column().

PS: do not want to add `values(). Who knows, this might be added natively in the next version of PHP language level support for ENUMs.
So i do not want tho shadow it.

@jkowalleck
Copy link
Member Author

closed by #266
release to be triggered soon.

@jkowalleck jkowalleck unpinned this issue Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants