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.

Sign up for GitHub

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

Jump to bottom

plugin: re-organize proposed api files #12550

Merged
merged 1 commit into from
Jun 23, 2023
Merged

plugin: re-organize proposed api files #12550

merged 1 commit into from
Jun 23, 2023

Conversation

vince-fugnitto
Copy link
Member

What it does

The pull-request re-organizes our theia-proposed.d.ts to follow the same conventions present in vscode, and also make it easier to update or add proposed apis in the future.

The changes:

  • split the proposed apis by sections into their own files
  • add the "extra" apis to theia-extra.d.ts properly

How to test

Review checklist

Reminder for reviewers

@vince-fugnitto vince-fugnitto added the vscode issues related to VSCode compatibility label May 19, 2023
@vince-fugnitto vince-fugnitto self-assigned this May 19, 2023
@alvsan09 alvsan09 self-requested a review May 25, 2023 13:30
alvsan09
alvsan09 approved these changes May 25, 2023
View reviewed changes
Copy link
Contributor

@alvsan09 alvsan09 left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I have reviewed the distribution of code to the different new files,
I found all code previously hosted in the former theia-proposed.d.ts file
mapped to the new files. 👍

The additions and modifications look good to me !

Testing was limited to start theia validate that no new activation failures arise,
and test some basic functionality.

@JonasHelming
Copy link
Contributor

@rschnekenbu FYI

rschnekenbu
rschnekenbu reviewed Jun 19, 2023
View reviewed changes
Copy link
Contributor

@rschnekenbu rschnekenbu left a comment

Choose a reason for hiding this comment

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

Same as @alvsan09, I did a quick test running theia and having no additional issues.
I have minor comments on textSearchQuery.d.ts and findTextInFile.d.ts proposals, which have duplicate code.
Otherwise, that looks very good to me and it's a great way to improve proposed APIs support!

packages/plugin/src/theia.proposed.textSearchQuery.d.ts Outdated Show resolved Hide resolved
packages/plugin/src/theia.proposed.textSearchQuery.d.ts Outdated Show resolved Hide resolved
packages/plugin/src/theia.proposed.findTextInFiles.d.ts Outdated Show resolved Hide resolved
@vince-fugnitto
Copy link
Member Author

@rschnekenbu thank you for the feedback, I updated the pull-request accordingly.

rschnekenbu
rschnekenbu approved these changes Jun 20, 2023
View reviewed changes
Copy link
Contributor

@rschnekenbu rschnekenbu left a comment

Choose a reason for hiding this comment

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

@vince-fugnitto, that looks good to me!

@tsmaeder tsmaeder merged commit 803efc9 into master Jun 23, 2023
@github-actions github-actions bot added this to the 1.39.0 milestone Jun 23, 2023
@vince-fugnitto vince-fugnitto deleted the vf/cleanup-proposed branch June 23, 2023 15:58
This was referenced Feb 11, 2024
Open
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rschnekenbu rschnekenbu rschnekenbu approved these changes

@alvsan09 alvsan09 alvsan09 approved these changes

@tsmaeder tsmaeder Awaiting requested review from tsmaeder

@marcdumais-work marcdumais-work Awaiting requested review from marcdumais-work

Assignees

@vince-fugnitto vince-fugnitto

Labels
vscode issues related to VSCode compatibility
Projects
PR Backlog
Archived in project
Milestone
1.39.0
Development

Successfully merging this pull request may close these issues.
5 participants
@vince-fugnitto @JonasHelming @rschnekenbu @alvsan09 @tsmaeder