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

Typo in License() parameter spxd_license_id - should be spdx_license_id #176

Closed
schlenk opened this issue Feb 18, 2022 · 4 comments
Closed
Labels
breaking change bug Something isn't working documentation Improvements or additions to documentation
Milestone

Comments

@schlenk
Copy link
Contributor

schlenk commented Feb 18, 2022

class License(builtins.object)
| License(spxd_license_id: Optional[str] = None

The parameter is prefixed by 'spxd' but the license page referenced is called "SPDX" (https://spdx.dev/)

Might be API breaking to change it.

Found in cyclonedx-python-lib 1.3.0

@schlenk schlenk changed the title Spelling of License spxd_license_id parameter is wrong, should probably be spdx_license_id Spelling of License spxd_license_id parameter is confusing, should probably be spdx_license_id Feb 18, 2022
@jkowalleck
Copy link
Member

jkowalleck commented Feb 19, 2022

Could you elaborate, @schlenk ?

What seams to be the issue? What is an acceptable solution? how would you suggest to solve the situation?

In addition, you could open a pull request to show a possible mitigation.
If you do so,

  1. please read https://github.com/CycloneDX/cyclonedx-python-lib/blob/main/CONTRIBUTING.md
  2. be aware that a breaking change is already in the making - so no problem if you intend to introduce breaking changes.
    • see RELEASE 2.0.0 #148
    • you could branch out from feat/add-bom-services
      and pull request into that branch.

@schlenk
Copy link
Contributor Author

schlenk commented Feb 19, 2022

The referenced standard is called "SPDX", so the license_id referenced in the id property and set by the constructor is a "spdx_license_id". But the constructor named parameter is called "spxd_license_id".

Calling the parameter "spxd_license_id" is confusing due to the typo (xd instead of dx) in the identifier.

If the API is broken with 2.0.0 anyway, i would simply rename the parameter to the better fitting one.

@jkowalleck jkowalleck added this to the 2.0.0 milestone Feb 19, 2022
@jkowalleck jkowalleck added breaking change bug Something isn't working documentation Improvements or additions to documentation labels Feb 19, 2022
@jkowalleck jkowalleck changed the title Spelling of License spxd_license_id parameter is confusing, should probably be spdx_license_id Typo in License() parameter spxd_license_id - should be spdx_license_id Feb 19, 2022
@jkowalleck
Copy link
Member

@madpah as we are planning to have most parameters forced to be keys, this typo should be addressed.
in fact, i see it was fixed already, in

def __init__(self, *, spdx_license_id: Optional[str] = None, license_name: Optional[str] = None,

via #160

therefore i marked the relevant PRs to close this very PR, after the fact.

@madpah
Copy link
Collaborator

madpah commented Feb 21, 2022

@madpah madpah closed this as completed Feb 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

No branches or pull requests

3 participants