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
docs(readme): adds deprecated warning and commands #955
Conversation
adds deprecated commands warning
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |
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.
lgtm
README.md
Outdated
@@ -57,6 +57,12 @@ Supporting developers is an important task for webpack CLI. Thus, webpack CLI pr | |||
- [`webpack-cli generate-loader`](./packages/generate-loader/README.md#webpack-cli-generate-loader) - Initiate new loader project. | |||
- [`webpack-cli serve`](./packages/serve/README.md#webpack-cli-serve) - Use webpack with a development server that provides live reloading. | |||
|
|||
> Deprecated commands as of v3.3.3 |
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.
Would it make sense to explicitly say Deprecated commands since v3.3.4
here
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.
I guess the current is fine. but it would be good if we say which version still supports those command. What do you think ?
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.
Might do a range instead <= 3.3.3
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.
Yeah....this one can be done 👍
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.
@anikethsaha Yeah, the intention is to make it easier for people to catch the change. Most of them (including me) will just skim the readme at first. It will be really catchy if we have specified the version in which the change came into effect.
I like @evenstensberg suggestion too.
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.
Yes, let's use 'since'
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.
We decided on since
right?
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.
Left a suggestion :)
Could you address changes here and rebase? |
41ee8ca
@pranshuchittora Thanks for your update. I labeled the Pull Request so reviewers will review it again. @rishabh3112 Please review the new changes. |
Are they removed or deprecated? |
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.
lgtm
Removed |
adds deprecated commands warning
What kind of change does this PR introduce?
docs