Skip to content

Refactor to use Promise#21582

Merged
bpasero merged 5 commits intomicrosoft:masterfrom
katainaka0503:refactoring
Mar 1, 2017
Merged

Refactor to use Promise#21582
bpasero merged 5 commits intomicrosoft:masterfrom
katainaka0503:refactoring

Conversation

@katainaka0503
Copy link
Contributor

@katainaka0503 katainaka0503 commented Mar 1, 2017

Refactored some files to use Promise.

@mention-bot
Copy link

mention-bot commented Mar 1, 2017

@katainaka0503, thanks for your PR! By analyzing the history of the files in this pull request, we identified @Tyriar and @bpasero to be potential reviewers.

@msftclas
Copy link

msftclas commented Mar 1, 2017

@katainaka0503,
Thanks for having already signed the Contribution License Agreement. Your agreement was validated by Microsoft. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

@bpasero bpasero self-assigned this Mar 1, 2017
@bpasero
Copy link
Member

bpasero commented Mar 1, 2017

@katainaka0503 looks like my last push introduced merge conflict, can you check?

@katainaka0503
Copy link
Contributor Author

katainaka0503 commented Mar 1, 2017

@bpasero fixed conflict.

@bpasero bpasero added this to the March 2017 milestone Mar 1, 2017
bpasero
bpasero requested changes Mar 1, 2017
View reviewed changes
Copy link
Member

@bpasero bpasero left a comment

Choose a reason for hiding this comment

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

Looks good to me, except for one minor thing.

src/vs/workbench/parts/git/node/git.lib.ts
});
return detectMimesFromStream(child.stdout, null).then(result =>
isBinaryMime(result.mimes) ?
TPromise.wrapError<string>(<IFileOperationResult>{
Copy link
Member

@bpasero bpasero Mar 1, 2017

Choose a reason for hiding this comment

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

@katainaka0503 return missing here?

Copy link
Contributor Author

@katainaka0503 katainaka0503 Mar 1, 2017

Choose a reason for hiding this comment

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

OK. I am fixing this. Is it better to write return explicitly?

@bpasero
Copy link
Member

bpasero commented Mar 1, 2017

@katainaka0503 builds are failing due to bad whitespace indentation (e.g. https://travis-ci.org/Microsoft/vscode/jobs/206537447), can you format your changes and submit again?

@katainaka0503 katainaka0503 force-pushed the refactoring branch 2 times, most recently from 7c6969f to 1b64840 Compare March 1, 2017 11:19
@bpasero bpasero merged commit 7f0f310 into microsoft:master Mar 1, 2017
@bpasero
Copy link
Member

bpasero commented Mar 1, 2017

Thanks 👍

@katainaka0503
Copy link
Contributor Author

katainaka0503 commented Mar 1, 2017

Thank you for the merge!

@katainaka0503 katainaka0503 deleted the refactoring branch March 1, 2017 11:41
@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.

Reviewers

@bpasero bpasero bpasero requested changes

Assignees

@bpasero bpasero

Labels

None yet

Projects

None yet

Milestone

March 2017

Development

Successfully merging this pull request may close these issues.

4 participants

@katainaka0503 @mention-bot @msftclas @bpasero