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

Theia 1.54.0 #402

Closed sgraband closed 3 weeks ago

sgraband commented 4 weeks ago

What it does

Update to Theia 1.54.0. Add Theia AI extensions. Adjust getting started to show AI disclaimer.

How to test

Review checklist

Reminder for reviewers

jfaltermeier commented 4 weeks ago

Regarding the failing license check. One of the identified issues is very likely just a bug in the tooling: https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/16197

The other failure is a bit weird, since there is this approval: https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/16326 The rerun opened another ticket: https://gitlab.eclipse.org/eclipsefdn/emo-team/iplab/-/issues/16335

I think there the database might not be updated yet.

So I think we should be good to merge, but maybe lets wait a bit, if we can, to see if a rerun later works.

sgraband commented 4 weeks ago

Now only the identified issue is left, right? Should we go ahead and merge this?

jfaltermeier commented 4 weeks ago

I’m not sure, to be honest.

The reported version came in with our last yarn upgrade, where the license check passed without issues. Our last green build on master is here: https://github.com/eclipse-theia/theia-blueprint/commit/7c072a654c143444eede4e4c459d6f27ead31fa8, and this version was included in that build as well.

However, since this commit: https://github.com/eclipse-theia/theia-blueprint/commit/41075bab17239f0a86f064bcced978b4d02911e7, the dependency is now being reported as an error.

So I can’t tell if the bug was that the dependency was allowed by the tooling initially or if the bug is that it’s no longer allowed. So we should probably wait.

marcdumais-work commented 3 weeks ago

I’m not sure, to be honest.

The reported version came in with our last yarn upgrade, where the license check passed without issues. Our last green build on master is here: 7c072a6, and this version was included in that build as well.

However, since this commit: 41075ba, the dependency is now being reported as an error.

So I can’t tell if the bug was that the dependency was allowed by the tooling initially or if the bug is that it’s no longer allowed. So we should probably wait.

We noticed similar strangeness about the same dependency / similar version, in one of our cdt.cloud repos. A PR that did not update the dependencies was the first to fail the license check. I am also not sure why, but in the past a similar thing happened when clearlydefined lowered the metric they provide, that reflect how sure they are about a dependency's license having been correctly detected. Dash-licenses uses that metric and has a threshold, below which it will flag the dependency for needing a review.

If you want the decision - to rightfully ignore that dependency's license check - documented in the repo, you could add a "license check exclusion" entry, such as this one (adapt to the version you use): https://github.com/eclipse-cdt-cloud/theia-trace-extension/blob/master/configs/license-check-exclusions.json#L6