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

Removed the leftover @theia/json dependency. #8182

Closed
wants to merge 3 commits into from

Conversation

kittaakos
Copy link
Contributor

@kittaakos kittaakos commented Jul 16, 2020

Signed-off-by: Akos Kitta [email protected]

NOTE: This PR was opened against #8151 and not the master.

What it does

It removes the @theia/json dependency from the test. I am not sure if this PR makes any sense. Can you please look into it, @akosyakov? Thank you!

Closes #8183

How to test

The CI should be green.

Review checklist

Reminder for reviewers

akosyakov added 2 commits July 8, 2020 09:16
as well fixes some exceptions caused by disposed widgets

Signed-off-by: Anton Kosyakov <[email protected]>
@kittaakos kittaakos requested a review from akosyakov July 16, 2020 09:00
@vince-fugnitto vince-fugnitto added ci issues related to CI / tests test issues related to unit and api tests labels Jul 16, 2020
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

The tests currently fail but it might be unrelated, we can either restart the CI or open the changes on top of #8151 which make the api integration suite more robust and fixes other issues.

@kittaakos
Copy link
Contributor Author

open the changes on top of #8151 which make the api integration suite more robust and fixes other issues.

Smart 👍 I'll do that.

@kittaakos kittaakos changed the base branch from master to akosyakov/some-browser-tests-do-8093 July 16, 2020 12:48
@kittaakos kittaakos force-pushed the remove-theia-json branch from fe64969 to 30b7a14 Compare July 16, 2020 12:52
Base automatically changed from akosyakov/some-browser-tests-do-8093 to master July 20, 2020 07:17
@akosyakov
Copy link
Member

Fine with me if tests are green.

@vince-fugnitto
Copy link
Member

@kittaakos when the base was changed it looked like travis didn't start (since it was not based off master), but the api-tests were recently merged. Do we want to rebase this pull-request to start travis?

@kittaakos
Copy link
Contributor Author

when the base was changed it looked like travis didn't start (since it was not based off master)

Yes, I can confirm.

but the api-tests were recently merged. Do we want to rebase this pull-request to start travis?

I'd leave the decision to @akosyakov.

@kittaakos kittaakos closed this Sep 11, 2020
@kittaakos kittaakos deleted the remove-theia-json branch September 15, 2020 06:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci issues related to CI / tests test issues related to unit and api tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[quality][json] Check why we depend on @theia/json in the TS tests
3 participants