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

Replaced some sync methods for their async version #291

Merged
merged 1 commit into from
Dec 4, 2019

Conversation

gndelia
Copy link
Contributor

@gndelia gndelia commented Oct 29, 2019

See #265

I removed most of the sync functions, there are some scenarios which I am unsure what to do. For instance, those sync calls that are at module level (like src/index.ts line 50 or line 60)

Same for src/errors/transformErrorMessages.ts - I am unsure if I can make that function async

Still pending one change from getProjectPath() in src/index.ts

@gndelia gndelia marked this pull request as ready for review November 5, 2019 21:48
@gndelia
Copy link
Contributor Author

gndelia commented Nov 5, 2019

There are some options I was unable to migrate - I think it's not possible. Like the following https://github.com/jaredpalmer/tsdx/blob/master/src/index.ts#L50 and a couple more that are code part of a module and therefore can't be async (that's my understanding)

@swyxio
Copy link
Collaborator

swyxio commented Nov 6, 2019

mm i think we had some merge conflicts from other PRs. if you have time to address them, be my guest, if not i'll try to update when i free up

@gndelia
Copy link
Contributor Author

gndelia commented Nov 6, 2019

@sw-yx I rebased and solve the conflicts :)

@swyxio swyxio merged commit c5f65bc into jaredpalmer:master Dec 4, 2019
@swyxio
Copy link
Collaborator

swyxio commented Dec 4, 2019

Thank you!

@agilgur5
Copy link
Collaborator

agilgur5 commented Mar 9, 2020

@all-contributors please add @gndelia for code

@allcontributors
Copy link
Contributor

@agilgur5

I've put up a pull request to add @gndelia! 🎉

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