-
-
Notifications
You must be signed in to change notification settings - Fork 601
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
Types for packages #578
Types for packages #578
Conversation
Add setup walk through and add usage description
Add setup walk through and add usage description
Help Needed: setting up webpack-cli globally
Add Property/keys section under each question and other minor changes
Change -D flag to --save-dev flag
Sync with webpack/webpack-cli
Do they actually exist those files? The |
They don't that's why this PR is for @ematipico |
What I am saying is that these files they don't exist inside the various packages (migrate folder, for example). So, referring to them inside the |
Have set |
If the whole codebase has been shifted to typescript |
Unfortunately we still need it I think, probably for the tests? I think @dhruvdutt can answer your question in a better way |
Some parts of the codebase like the entrypoint folder is still in JS but this shouldn't affect the TS compilation process. We can remove probably About the Also, just a friendly note to not override the PR template. You don't need to fill it completely, you can ignore the questions/fields which aren't relevant to your PR but please keep the template consistent. 🙂 |
Why? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a small comment on commit format. type(scope): msg
. In order for tests to pass. Important to follow convention so that the code doesn't get messy
Members and Contributors, |
@ev1stensberg: @rishabh3112: You can definitely work on this. I think it needs some inputs from my end, but I'm always open to review/suggest. :) |
Okay, @dhruvdutt. |
@rishabh3112 Thanks for your update. I labeled the Pull Request so reviewers will review it again. @ematipico Please review the new changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the reason for declarations
suffix?
As it is used when declaration of ts files is needed (i.e. ts files in packages) @ev1stensberg |
Could it be done without the suffix? |
Yes. |
Maybe |
Why this PR was closed @ematipico? |
Apologies! I might have hit the wrong button |
@ematipico then should I proceed with your suggestion? |
@ev1stensberg we need to have a specific configuration only for those packages that need to expose the interfaces, so we need to keep separated configurations. About the naming, yes let's call it with |
Oki :) |
@rishabh3112 The most important CI builds failed. This way your PR can't be merged. Please take a look at the CI results from travis (failure) and fix these issues. |
Do I need to rebase here for commitlint to pass or you may reference the latest commit @ematipico? |
@rishabh3112 No need to stress about the commitlint. We'll rewrite the message during merge. We got it covered. Can you please fix the conflicts? |
PR is good to go :) no conflicts |
As far as i can see it says |
refers #577