docs(contributing): improve formatting#768
Conversation
|
Could you please remove the third bullet point? It's old documentation and code |
|
Hi @ematipico, I've removed the third bullet point, now the setup section looks like this: |
evenstensberg
left a comment
There was a problem hiding this comment.
lgtm, thanks for submitting a PR!
evenstensberg
left a comment
There was a problem hiding this comment.
Could you rebase against master and fix any formatting errors? Thanks!
|
@misterdev Thanks for your update. I labeled the Pull Request so reviewers will review it again. @evenstensberg Please review the new changes. |
|
There were the following issues with this Pull Request
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! |
|
There were the following issues with this Pull Request
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! |
0a41fb9 to
94387e7
Compare
|
|
||
| * To test a single CLI (other type of) test case: | ||
| - `yarn jest path/to/my-test.js` | ||
| * You can also install jest globally and run tests without npx: |
There was a problem hiding this comment.
I wouldn't suggest to install a package globally. Npx is enough as it comes for free and there's no installation process
There was a problem hiding this comment.
So why it is marked as an addition in this PR? Did you rebase your branch?
There was a problem hiding this comment.
It's for people that want to reuse a scaffold @ematipico instead of re-installing every time. I don't think npx will completely solve that issue
There was a problem hiding this comment.
IMO Reusing a scaffold is related to @webpack-cli/init being able work with it's global installation @evenstensberg.
I think for this issue we should just write if the user has global installation of jest then we should run jest without npm for test.
There was a problem hiding this comment.
@ematipico I moved this code to separate setup and testing, it is marked as deleted some lines above
410fe38 to
1042cb2
Compare
|
Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon. |

What kind of change does this PR introduce?
Improves formatting of the Setup instructions in CONTRIBUTING.md
Before:

After:
