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

feat: promisify install #2222

Closed
wants to merge 3 commits into from
Closed

Conversation

imatlopez
Copy link
Contributor

@imatlopez imatlopez commented Sep 15, 2020

Checklist
  • npm install && npm test passes
  • tests are included
  • commit message follows commit guidelines
Description of change

An effort to modernize the codebase, handover control of installation
to promises.

cc: @rvagg

@imatlopez imatlopez marked this pull request as ready for review September 15, 2020 01:44
@rvagg
Copy link
Member

rvagg commented Nov 3, 2020

@imatlopez would you mind trying to rebase your branches rather than merging master back in? when they land they'll need to be merge-commit-free which will end up requiring awkward cherry-picking and conflict resolving. If you could do that early then it might save work later on.

I'm also thinking about how to bite all of this off. I'm keen to get some of this work in but it's (a) a lot of work to review carefully, so takes time, and (b) high-risk, as with any major change here since our test suite is so sparse but the user-base so massive. So I hope you understand the delays here, I just wish I had a clear strategy for it. Maybe we should try something like merging into a next branch and doing pre-releases or something. It's difficult to get real use of versions that aren't bundled with npm, but perhaps we could try at least.

There's also #2140 which is in a similar limbo and has similar change scope.

An effort to modernize the codebase, handover control of installation
to promises.
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.

None yet

2 participants