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

feat: adjust set command to support version ranges #146

Merged
merged 94 commits into from
Jul 2, 2024
Merged

Conversation

CBeck-96
Copy link
Collaborator

Expand set to allow the processing for version ranges.

@github-actions github-actions bot added documentation Improvements or additions to documentation enhancement New feature or request settings_changes unittests labels Mar 27, 2024
Copy link
Contributor

github-actions bot commented Mar 27, 2024

Coverage

Coverage Report •
FileStmtsMissCoverMissing
__main__.py3232093%217–218, 235, 245, 664–665, 669–674, 676, 679, 689–692, 696, 861
set.py1973781%56–59, 78–86, 88–89, 91–94, 105, 196–197, 200, 202, 205, 209, 213–217, 236, 264, 273, 304, 307, 334
TOTAL16779194% 

Tests Skipped Failures Errors Time
295 2 💤 0 ❌ 0 🔥 4.922s ⏱️

@CBeck-96 CBeck-96 requested a review from mmarseu March 28, 2024 07:55
@italvi italvi changed the title Feat: add support for version ranges to set feat: adjust set command to support version ranges Apr 4, 2024
Copy link
Collaborator

@mmarseu mmarseu left a comment

Choose a reason for hiding this comment

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

I suggest you don't attempt to invent your own version range syntax. There are smarter people than us working on that sort of thing and they came up with existing solutions, which additionally fit nicely into the CycloneDX ecosystem. Primarily, purl's proposed version range specification comes to mind:

It is already being referenced by CycloneDX

This would largely render version_processing.py obsolete and therefore cuts down the code in this PR by more than half. Its always great to have less code to maintain!

Copy link
Collaborator

@mmarseu mmarseu left a comment

Choose a reason for hiding this comment

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

This is a lot better than the last attempt. Good job!
A few comment remain but mostly minor stuff.

@CBeck-96 CBeck-96 force-pushed the set_version_ranges branch 4 times, most recently from 6944a21 to 3fcc463 Compare June 24, 2024 19:49
@CBeck-96 CBeck-96 requested a review from mmarseu June 24, 2024 20:17
@mmarseu
Copy link
Collaborator

mmarseu commented Jun 25, 2024

Is it intentional, that a version range cannot be set via the command-line, only using the --from-file option?

Copy link
Collaborator

@mmarseu mmarseu left a comment

Choose a reason for hiding this comment

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

Almost good to go!

@mmarseu mmarseu mentioned this pull request Jun 25, 2024
5 tasks
@CBeck-96
Copy link
Collaborator Author

Is it intentional, that a version range cannot be set via the command-line, only using the --from-file option?

It was not...

@CBeck-96 CBeck-96 requested a review from mmarseu June 25, 2024 19:14
mmarseu
mmarseu previously approved these changes Jun 26, 2024
Copy link
Collaborator

@italvi italvi left a comment

Choose a reason for hiding this comment

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

@CBeck-96 please change the flag according to our guidelines and adjust your example, so that everybody understands when the version_range would apply or not, as otherwise everybody would be required to read the univers-specification.

Further, please check if the use of the term "schema" is really required or you could stick to only use "range".

@CBeck-96 CBeck-96 force-pushed the set_version_ranges branch from 2d38fbd to 5174308 Compare July 1, 2024 09:03
@CBeck-96 CBeck-96 requested a review from italvi July 1, 2024 09:14
@italvi italvi requested a review from mmarseu July 1, 2024 09:27
@CBeck-96 CBeck-96 force-pushed the set_version_ranges branch from ff226af to 6a163ed Compare July 1, 2024 09:52
@CBeck-96 CBeck-96 requested a review from mmarseu July 1, 2024 14:27
mmarseu
mmarseu previously approved these changes Jul 2, 2024
@italvi italvi merged commit 6b32db5 into main Jul 2, 2024
5 checks passed
@italvi italvi deleted the set_version_ranges branch July 2, 2024 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request settings_changes unittests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants