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

8969-upgrade-monaco-editor-core-to-standalone-0.22.x #9081

Conversation

danarad05
Copy link
Contributor

@danarad05 danarad05 commented Feb 17, 2021

In preparation for upgrading monaco-editor-core to standalone/0.22.x these changes were implemented:

  1. signatures alignment to standalone/0.22.x
  2. editor preferences synced with standalone/0.22.x

Signed-off-by: Dan Arad dan.arad@sap.com

What it does

This PR updates Theia repo with these changes:

  1. signatures alignment to standalone/0.22.x
  2. editor preferences synced with standalone/0.22.x

It is also second part of 8969 where first part is generating monaco-editor-core@0.22.

How to test

  1. How to test section for the previous PR Upgrade to Monaco 0.20.0 #8010.
  2. Integration tests in: add integration tests for typescript language #7265.

Review checklist

Reminder for reviewers

In preparation for upgrading monaco-editor-core to standalone/0.22.x these changes were implemented:
1) signatures alignment to standalone/0.22.x
2) editor preferences synced with standalone/0.22.x

Signed-off-by: Dan Arad <dan.arad@sap.com>
Copy link
Member

@paul-marechal paul-marechal left a comment

Choose a reason for hiding this comment

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

LGTM, CI failure is unrelated.

@paul-marechal
Copy link
Member

paul-marechal commented Mar 3, 2021

@danarad05 I was wondering if there's any reason why we can't use things from vscode's monaco editor rather than copying?

Copy link
Contributor

@RomanNikitenko RomanNikitenko left a comment

Choose a reason for hiding this comment

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

I guess the PR is only one part of #8969
and the changes should be tested and applied when a new version of @theia/monaco-editor-core is applied to Theia.

I mean - at the moment Theia uses 0.20.0 version of Monaco, but the current changes for 0.22 version.

@danarad05
should it be a draft PR or maybe I missed something...

@danarad05
Copy link
Contributor Author

danarad05 commented Mar 4, 2021

@RomanNikitenko

I guess the PR is only one part of #8969
and the changes should be tested and applied when a new version of @theia/monaco-editor-core is applied to Theia.

I mean - at the moment Theia uses 0.20.0 version of Monaco, but the current changes for 0.22 version.

correct. this PR is second part of new 0.22 version - whose repo location is currently debated.

should it be a draft PR or maybe I missed something...

If that it the procedure then it should be draft.. don't remember if there was an option. can you change type to draft? I'm unable.
Thanks

@danarad05
Copy link
Contributor Author

danarad05 commented Mar 4, 2021

@marechal-p Thanks for reviewing.

wondering if there's any reason why we can't use things from vscode's monaco editor rather than copying?

That is a good question. I'm not sure what are reasons for this approach - currently there aren't many diffs - maybe in past there were more.
@RomanNikitenko - do you know?

@RomanNikitenko
Copy link
Contributor

If that it the procedure then it should be draft.. don't remember if there was an option. can you change type to draft? I'm unable.

I meant - it's better to have the draft status for the current state of the PR, so people can see that it's still in progress.

It should be possible to change a PR status from Reviewers menu => Convert to draft
https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/changing-the-stage-of-a-pull-request#converting-a-pull-request-to-a-draft

@RomanNikitenko
Copy link
Contributor

@marechal-p Thanks for reviewing.

wondering if there's any reason why we can't use things from vscode's monaco editor rather than copying?

That is a good question. I'm not sure what are reasons for this approach - currently there aren't many diffs - maybe in past there were more.
@RomanNikitenko - do you know?

You can see some info about it here:

@danarad05 danarad05 marked this pull request as draft March 4, 2021 13:09
@danarad05
Copy link
Contributor Author

If that it the procedure then it should be draft.. don't remember if there was an option. can you change type to draft? I'm unable.

I meant - it's better to have the draft status for the current state of the PR, so people can see that it's still in progress.

It should be possible to change a PR status from Reviewers menu => Convert to draft
https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/changing-the-stage-of-a-pull-request#converting-a-pull-request-to-a-draft

Don't know how I missed it:).. Thanks

@danarad05 danarad05 changed the title 8969-upgrade-monaco-editor-core-to-standalone-0.22.x 8969-upgrade-monaco-editor-core-to-standalone-0.23.x Mar 7, 2021
@danarad05 danarad05 changed the title 8969-upgrade-monaco-editor-core-to-standalone-0.23.x 8969-upgrade-monaco-editor-core-to-standalone-0.22.x Mar 7, 2021
@danarad05
Copy link
Contributor Author

Closing this PR as created a new one - #9154 - for version standalone/0.23.x which has been released in meantime.

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.

3 participants