Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

BugFix : Removing case sensitivity of licenseIds - #784

#784
Closed
puneeth072003 wants to merge 2 commits into
spdx:mainspdx/tools-python:mainfrom
puneeth072003:mainpuneeth072003/tools-python:mainCopy head branch name to clipboard
Closed

BugFix : Removing case sensitivity of licenseIds#784
puneeth072003 wants to merge 2 commits into
spdx:mainspdx/tools-python:mainfrom
puneeth072003:mainpuneeth072003/tools-python:mainCopy head branch name to clipboard

Conversation

@puneeth072003

Copy link
Copy Markdown

This PR fixes the issue #165 from ntia-conformance-checker repo.

Signed-off-by: Puneeth <pyd773@gmail.com>

validation_messages = []
license_ref_ids: List[str] = [license_ref.license_id for license_ref in document.extracted_licensing_info]
license_ref_ids: List[str] = [license_ref.license_id.lower() for license_ref in document.extracted_licensing_info]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

going to .lower() feels wrong, as the upper case is the default for most licenses?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Should I take any other approach to make licenseIds case insensitive.(we have couple of other methods other than .lower())

Or keeping them case sensitive is just fine:(

Well, what are your thoughts?

@puneeth072003 puneeth072003 Jan 2, 2024

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Sorry, I'm new to licenses and stuff.

It would be great to have your opinion

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think uppercase would be more consistent

@maxhbr

maxhbr commented Jan 2, 2024

Copy link
Copy Markdown
Member

Can you please also add tests to show and test the new behaviour?

@maxhbr

maxhbr commented Jan 2, 2024

Copy link
Copy Markdown
Member

As this is also relevant for matching and relating licenseRefs, I assume that this also needs changes beyond the validation layer.

@puneeth072003

puneeth072003 commented Jan 2, 2024

Copy link
Copy Markdown
Author

Can you please also add tests to show and test the new behaviour?

Sure....but it can take a while
Tad busy at the moment

I'll add them by this weekend

@maxhbr

maxhbr commented Mar 22, 2024

Copy link
Copy Markdown
Member

@puneeth072003 , do you plan to continue on that topic? I think this is a helpful and important feature, but a consistent and complete implementation is necessary

@puneeth072003

Copy link
Copy Markdown
Author

@maxhbr sure I'd like to continue

@armintaenzertng

Copy link
Copy Markdown
Collaborator

Hi @puneeth072003, are you still working on this? Otherwise I can take over from you if you like.

@puneeth072003

puneeth072003 commented Oct 7, 2024

Copy link
Copy Markdown
Author

@armintaenzertng yep sure! I'm lil preoccupied at the moment you can take over this one

@rtabassum

rtabassum commented Oct 28, 2024

Copy link
Copy Markdown

Hi, Is there any update on this one? Was hoping to get a fix for the same issue: #810

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 this pull request may close these issues.

4 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.