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

#248 Fix Codicons dependency #250

Merged
merged 1 commit into from
Oct 21, 2021
Merged

#248 Fix Codicons dependency #250

merged 1 commit into from
Oct 21, 2021

Conversation

ndoschek
Copy link
Contributor

Move "@vscode/codicons" from devDependencies to dependencies.

Copy link
Contributor

@planger planger left a comment

Choose a reason for hiding this comment

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

Thanks, looks good to me! 👍

@planger planger merged commit e13c794 into eclipse-sprotty:master Oct 21, 2021
@ndoschek ndoschek deleted the fix-codicons-dependency branch October 21, 2021 09:14
@spoenemann
Copy link
Contributor

I'm thinking about whether we need to file a CQ for this dependency because it's CC-licensed. But since Theia is using it as well, it's probably not necessary. WDYT?

@tortmayr
Copy link
Contributor

AFAIK the Eclipse foundation has changed their IP Policy and in general CQs are no longer required for third-party contents.
The corresponding section of the project handbook states the following:

  • CQs are not required for third-party content in all cases.
  • CQs are no longer required before third-party content is introduced.

So I think this PR is fine as is.

@spoenemann
Copy link
Contributor

CQs are not required for third-party content in all cases.

That does not mean "CQs are not required in any case". There are still cases where you should file a CQ, for example if the license compatibility is uncertain.

@tortmayr
Copy link
Contributor

I see, I misinterpreted that.
I had a loot at the PR that introduced the dependency in Theia. Apparently CC is part of the list of approved third party licenses so no CQ is required here.

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