-
Notifications
You must be signed in to change notification settings - Fork 2k
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
Fix license metadata #3233
Fix license metadata #3233
Conversation
While we're doing this, may I ask that we also change inso to use a valid license? Right now it's @nijikokun is that ok? In case you think this is a pedantic request, some tooling reports this when you install, e.g.:
And, if nothing else, it'd be nice to have that error go away. |
Thanks for raising the issue @dimitropoulos. Let's go ahead and make sure it's properly labeled as |
Done 👍 |
I am not sure whether the changes in this PR will actually resolve #3219, as it is unclear whether actually having the license in the That said, I still think we should merge the changes in this PR regardless, but I'm not sure if we can close #3219 until we verify the resulting |
I tested it locally and putting the license field in Just to be safe I can test it again |
@DMarby Just checked it locally and yes, both .rpm and .deb build with the correct license. However I'm not familiar with metadata to .AppImage and .snap but I can look into it too |
Digging deeper I've found that the .snap does not ship with a license field inside But solving this maybe out of scope of this PR |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll need to double check whether we should be applying MIT or Apache-2.0; to my knowledge it is the latter, so let's hold off on merging this until confirmed.
I echo the other points raised, however, that we should definitely get this in. Thank you for raising a PR and running through those investigations! 🤗
A quick update, this is on the backlog to chase up with the right people. Until then, marking as blocked 🙂 |
Okay! I have some more information:
@reynolek please advise if this doesn't capture it correctly 🙂 |
That looks correct from the info that I was given 👍 |
Alright, just applied the changes, please review |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Approving from product perspective based on Legal team's input
Thank you for working through this! |
Adds license field in packages so that linux builds ship with the correct license.
Closes #3219, Closes INS-599.