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: create-webpack-app #3036

Draft
wants to merge 19 commits into
base: master
Choose a base branch
from
Draft

feat: create-webpack-app #3036

wants to merge 19 commits into from

Conversation

rishabh3112
Copy link
Member

What kind of change does this PR introduce?

Did you add tests for your changes?

If relevant, did you update the documentation?

Summary

Does this PR introduce a breaking change?

Other information

@codecov
Copy link

codecov bot commented Nov 29, 2021

Codecov Report

Merging #3036 (681bf69) into master (66fbf56) will increase coverage by 0.51%.
The diff coverage is n/a.

❗ Current head 681bf69 differs from pull request most recent head d1ef790. Consider uploading reports for the commit d1ef790 to get more accurate results

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3036      +/-   ##
==========================================
+ Coverage   90.98%   91.50%   +0.51%     
==========================================
  Files          23       23              
  Lines        1731     1719      -12     
  Branches      519      519              
==========================================
- Hits         1575     1573       -2     
+ Misses        156      146      -10     
Impacted Files Coverage Δ
packages/webpack-cli/src/webpack-cli.ts 94.04% <0.00%> (+0.09%) ⬆️
packages/generators/src/index.ts 100.00% <0.00%> (+20.45%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66fbf56...d1ef790. Read the comment docs.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Can we add smoke test? Yes, it will be failed here, but I am do first release after this and we will look at tests

@rishabh3112
Copy link
Member Author

Yup sure, will do

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.

I don't see the point of naming an internal command/tool that is invoked through webpack-cli to something that indicates being a standalone tool..

@alexander-akait
Copy link
Member

@evenstensberg Earlier I described the problem, shorty - when you use npx webpack init, it is not work (try to use it), because webpack-cli is not a part of webpack and npm doesn't install it (so you got not found package error), so we need to use other name, to provide better DX

@rishabh3112
Copy link
Member Author

@alexander-akait should we wait for #2862 to get merged here?
Reason: It would be a difficult merge else as it has refactors in handler logic as well.

@alexander-akait
Copy link
Member

Reviewed 👍

@rishabh3112
Copy link
Member Author

Reviewed 👍

Thanks!

@rishabh3112
Copy link
Member Author

As #2862 is merged, I will continue working on this now!

@rishabh3112
Copy link
Member Author

rishabh3112 commented Feb 25, 2022

[Note for self] TODO: Move new templates into create webpack app repo.

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.

@rishabh3112 could you pick up the work on this again?

@rishabh3112
Copy link
Member Author

@evenstensberg there roadmap for the same currently? If not lets finalize that first. This PR is anyway very old now.

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.

None yet

4 participants