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

Remove PPromise #53487

Merged
merged 24 commits into from
Aug 7, 2018
Merged

Remove PPromise #53487

merged 24 commits into from
Aug 7, 2018

Conversation

joaomoreno
Copy link
Member

@joaomoreno joaomoreno commented Jul 3, 2018

This PR aims to remove the usage of progress of WinJS Promises.

The first commit removes the type definitions for progress, so this branch will remain broken until we remove all its usage.

@joaomoreno joaomoreno added debt Code quality issues engineering VS Code - Build / issue tracking / etc. labels Jul 3, 2018
@joaomoreno joaomoreno added this to the July 2018 milestone Jul 3, 2018
@joaomoreno joaomoreno assigned bpasero and dbaeumer and unassigned jrieken Jul 4, 2018
@joaomoreno
Copy link
Member Author

Down to 38 errors only... I've updated the first comment with checkboxes on the areas left to adopt. Please go through your area and replace usages of promise progress with simple callbacks.

@jrieken jrieken mentioned this pull request Jul 4, 2018
4 tasks
@joaomoreno
Copy link
Member Author

Removed Search IPC PPromise usage, awaiting review: #53536 cc @roblourens @chrmarti

joaomoreno added a commit that referenced this pull request Jul 5, 2018
joaomoreno added a commit that referenced this pull request Jul 5, 2018
@jens1o
Copy link
Contributor

jens1o commented Jul 6, 2018

What will be the advantage of this?

@joaomoreno
Copy link
Member Author

@jens1o This is just engineering debt. More details: #53526

@joaomoreno
Copy link
Member Author

Created #53875 for the task world. Search is all that's left.

@roblourens
Copy link
Member

Filed #53887 on myself

@connorshea
Copy link
Contributor

so is removing it from search done now 🤔

@roblourens
Copy link
Member

roblourens commented Aug 2, 2018

👍

image

@joaomoreno
Copy link
Member Author

Sweet! Moving this to next week.

@joaomoreno
Copy link
Member Author

@roblourens This is the last PPromise usage:

https://github.com/Microsoft/vscode/blob/remove-ppromise/src/vs/workbench/api/node/extHostSearch.fileIndex.ts#L419

Is it as simple as removing , onBatch from that then call?

@roblourens
Copy link
Member

Oops, yes done

@joaomoreno joaomoreno closed this Aug 7, 2018
@joaomoreno joaomoreno reopened this Aug 7, 2018
@joaomoreno
Copy link
Member Author

Thanks for all the help guys! 🎆

@joaomoreno joaomoreno merged commit c283ad9 into master Aug 7, 2018
@joaomoreno joaomoreno deleted the remove-ppromise branch August 7, 2018 14:18
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
debt Code quality issues engineering VS Code - Build / issue tracking / etc.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants