search

eclipse-theia / theia-ide

The Eclipse IDE is a modern and open IDE for cloud and desktop. The Theia IDE is based on the Theia platform. The Theia IDE is available as a downloadable desktop application. You can also try the latest version of the Theia IDE online. For more details, see the Readme below.
https://theia-ide.org/#theiaide
MIT License
343 stars 129 forks source link

Integrate License Check #334

Closed jfaltermeier closed 7 months ago

jfaltermeier commented 7 months ago

What it does

Contributed on behalf of STMicroelectronics

How to test

Check workflow on this PR Run script locally using yarn license:check

Review checklist

Reminder for reviewers

jfaltermeier commented 7 months ago

I've generated a PAT, however I don't have the admin rights to add secrets on the repository

marcdumais-work commented 7 months ago

I've generated a PAT, however I don't have the admin rights to add secrets on the repository

Hi @jfaltermeier , a PAT is not necessary at first I think(*). But if you are a committer in a repo, you can set a secret using the GitHub API. I've been using an old version of this docker appliance to do so myself.

(*): for security reasons, GitHub will only allow a secret be used for PRs that originate from this repo, not forks, so the "auto-review" mode will in any case not always be available.

jfaltermeier commented 7 months ago

Thanks, I've added the PAT and it seems to be working: https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/?sort=created_date&state=opened&author_username=jfaltermeier&in=DESCRIPTION&search=ecd.theia&first_page_size=20 Now we have to wait for the reviews to conclude

marcdumais-work commented 7 months ago

Now we have to wait for the reviews to conclude

It's ok and probably cleaner to wait - it will avoid having the new check fail on newly opened PRs and nightly builds. However, I wanted to mention that I believe that you are now dealing with the backlog, of dependencies that are already part of this repository and do not pass the dash-licenses check, rather than dependencies that are added through this PR, and so I think waiting is optional.

marcdumais-work commented 7 months ago

FYI, IP Tickets for 3PPs like @types/lodash.union/4.6.9 don't seem to be handled too well by the "license check bot" on EF Gitlab. I think such package's sources are hosted in a huge repo (DefinitelyTyped), along with thousands(*) of other similar packages for other 3PPs. The bot seemingly can't figure-out how to get the sources archive corresponding to these individual components and so can't analyse them.

It's almost certain there is no IP issue with such a @types package, but since the bot falls short, manual intervention from the IP team is needed. For such cases, you could optionally use the exclusion feature of the nodejs wrapper, so they would be ignored in the final license check assessment (i.e. not fail the check).

(*) https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types

The GitHub UI does not permit seeing them all: image

marcdumais-work commented 7 months ago

It's ok and probably cleaner to wait

Another option, that I am taking in one of my recent PR, is to use the "exclusion" feature of the dash-licenses wrapper, to ignore some of the dependencies that fail the license check. This way the license check momentarily passes and will be useful to detect further components with unclear licenses, while the few outstanding dependencies are analysed by the IP team.

IMHO, one needs to reasonably believe that the components, so excluded from the license check, are of an appropriate license for the project, and are being or to be reviewed by the IP team.