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

Add hover text and explanation of red/blue text #178

Merged
merged 1 commit into from
Dec 29, 2023
Merged

Add hover text and explanation of red/blue text #178

merged 1 commit into from
Dec 29, 2023

Conversation

goneall
Copy link
Member

@goneall goneall commented Dec 22, 2023

Fixes #165

Here's a screenshot of the current implementation:

image

Copy link
Member

@swinslow swinslow left a comment

Choose a reason for hiding this comment

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

LGTM, thank you @goneall!

@goneall
Copy link
Member Author

goneall commented Dec 24, 2023

Thanks for the review @swinslow

I'll wait a couple more days to see if @recursivenomad has any comments before merging.

@recursivenomad
Copy link

Thanks for holding for my input, @goneall! End of the year can be busy :)

I like it!

The technically curious side of me still feels that linking to a source for the specific text of each license (such as the XML) in a footnote might be useful just in case there are edge cases in the future where a user wishes to clarify something about the technical definition which is not captured on this page - but I acknowledge that that is future-proofing beyond the scope of the the original issue, and this PR certainly does resolve #165!

So if you're good with what's noted above, then it looks good to me to resolve 👍

@goneall
Copy link
Member Author

goneall commented Dec 29, 2023

Thanks @recursivenomad for the review.

The technically curious side of me still feels that linking to a source for the specific text of each license (#165 (comment)) in a footnote might be useful just in case there are edge cases in the future where a user wishes to clarify something about the technical definition which is not captured on this page

I agree - it's just more technical work since it would involve changes to both the template and several Java files in the back end to pass through the template file (although one could kind of hack the file name based on the license name).

I'll go ahead and merge this. If anyone is interested in creating a separate pull request to add the link to the XML file, that would be great.

@goneall goneall merged commit f9298f0 into master Dec 29, 2023
1 check passed
@goneall goneall deleted the issue165 branch December 29, 2023 18:23
@jlovejoy
Copy link
Member

@goneall - this has the issue of the link going to a specific spot in the v2.3 spec... should we change to go to https://github.com/spdx/license-list-XML/blob/main/DOCS/license-matching-guidelines-and-templates.md (which may also have different numbering) before this next release?

@jlovejoy
Copy link
Member

is there a hover-over for replaceable text implemented in this too??

@goneall
Copy link
Member Author

goneall commented Dec 18, 2024

is there a hover-over for replaceable text implemented in this too??

Yes - this PR includes a hover-over for both

@goneall
Copy link
Member Author

goneall commented Dec 18, 2024

is has the issue of the link going to a specific spot in the v2.3 spec... should we change to go to https://github.com/spdx/license-list-XML/blob/main/DOCS/license-matching-guidelines-and-templates.md (which may also have different numbering) before this next release?

It should be replaced, but I just did the release yesterday - so let's pick this up on the next release.

@swinslow - are you going to be making changes to the templates? If so, do you mind adding this to your list?

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.

Online license list omits important limitations
5 participants