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

fix(init): fix init prompt installation #742

Merged
merged 6 commits into from
Mar 10, 2019

Conversation

rishabh3112
Copy link
Member

What kind of change does this PR introduce?

bugfix
Did you add tests for your changes?
N/A
If relevant, did you update the documentation?
N/A
Summary
#741

Does this PR introduce a breaking change?

No
Other information

Copy link
Member

@evenstensberg evenstensberg left a comment

Choose a reason for hiding this comment

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

You can see how we did this at the next branch

@rishabh3112
Copy link
Member Author

Hi @evenstensberg, next branch has refactored bin into lib, can you point me where to find installation prompt ?

@evenstensberg
Copy link
Member

You can navigate the lib folder, it's not that big, and you learn the new internals

@rishabh3112
Copy link
Member Author

rishabh3112 commented Feb 4, 2019

@evenstensberg we don't do a global installation of @webpack-cli/init as per next branch instead forcing users to do a local installation. So next branch code doesn't encounter the error this PR supposed to solve. So there are two solutions:

  1. revert global installation setup to local for @webpack-cli/init as in next branch ( I don't like this personally)
  2. Get location to the global @webpack-cli/init package by getting prefix programatically using npm.

Which one I should implement?

@evenstensberg
Copy link
Member

Do as 1. Does it and ill review

@webpack-bot
Copy link

@rishabh3112 Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@evenstensberg Please review the new changes.

@rishabh3112
Copy link
Member Author

Did that @evenstensberg

@rishabh3112
Copy link
Member Author

rishabh3112 commented Feb 16, 2019

Any review here 😅?
@evenstensberg

@rishabh3112
Copy link
Member Author

Done @evenstensberg

@ghost
Copy link

ghost commented Mar 9, 2019

There were the following issues with this Pull Request

  • Commit: f9c130d
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

1 similar comment
@ghost
Copy link

ghost commented Mar 9, 2019

There were the following issues with this Pull Request

  • Commit: f9c130d
    • ✖ message may not be empty
    • ✖ type may not be empty

You may need to change the commit messages to comply with the repository contributing guidelines.


🤖 This comment was generated by commitlint[bot]. Please report issues here.

Happy coding!

@rishabh3112
Copy link
Member Author

rishabh3112 commented Mar 9, 2019

@evenstensberg after rebase with master NodeJS stable is failing on this PR.
Screenshot from 2019-03-09 15-39-18
failing on master too.
I have fixed commit messages here and this PR ready for review.

@rishabh3112
Copy link
Member Author

@evenstensberg After investigation error is certainly due to jest-util package having the error as per Node v11.11.0 (stable). it's working fine at lower node versions.

pasting error for the reference.

TypeError: Cannot assign to read only property 'Symbol(Symbol.toStringTag)' of object '#<process>'
      at exports.default (node_modules/jest-util/build/create_process_object.js:15:34)

@evenstensberg
Copy link
Member

Any fix to this?

@rishabh3112
Copy link
Member Author

rishabh3112 commented Mar 10, 2019

@evenstensberg This is fixed at jest.

@rishabh3112
Copy link
Member Author

Updating jest to v24.3.0 would fix this.

@rishabh3112 rishabh3112 reopened this Mar 10, 2019
@webpack-bot
Copy link

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

@evenstensberg evenstensberg merged commit 6aec4c3 into webpack:master Mar 10, 2019
@rishabh3112 rishabh3112 deleted the fix/init branch December 25, 2019 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants