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.

Sign up for GitHub

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: Added steps for using npm/npx locally to CONTRIBUTING.md #6643

Merged
merged 5 commits into from
Jul 17, 2023
Merged

docs: Added steps for using npm/npx locally to CONTRIBUTING.md #6643

Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
37ee6e3
Added steps for using npm/npx locally
rahulio96 Jul 13, 2023
37bcb30
revised npm/npx locally section replacing alias with node .
AaronHamilton965 Jul 17, 2023
3f5ad2d
Changed the examples to npm exec instead of npm install
rahulio96 Jul 17, 2023
062babb
Made example commands broader
rahulio96 Jul 17, 2023
7b92e76
Removed Run npm/npx Locally header
rahulio96 Jul 17, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 26 additions & 0 deletions CONTRIBUTING.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,32 @@ We use [Conventional Commits](https://www.conventionalcommits.org/en/v1.0.0/).

We use [`tap`](https://node-tap.org/) for testing & expect that every new feature or bug fix comes with corresponding tests that validate the solutions. Tap also reports on code coverage and it will fail if that drops below 100%.

## Run npm/npx Locally

If a specific command is not covered by tap, try the following:
Copy link
Member

@wraithgar wraithgar Jul 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think this line can be removed too. The advice is good for general testing if anything you're doing locally when developing, even long before you get to tests. Everything else looks good!

Copy link
Contributor

Choose a reason for hiding this comment

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

Okay, just removed that section, thanks for all your help!


To run your repository's version of the npm cli on your local machine use the following commands:

**npm commands:**
```bash
node . <command>
```

**npx commands:**
```bash
node . exec
```

For example instead of:
```bash
npm exec --yes false -- esbuild
Copy link
Member

Choose a reason for hiding this comment

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

What's the thinking behind forcing yes to be false?

Copy link
Contributor

Choose a reason for hiding this comment

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

Forcing yes to be false is not really necessary, as this is just an example command. We included it because it was a reference to the last issue we worked on.

Would you prefer if we made the command broader and changed it to: npm exec -- <package> and node . exec -- <package> ?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah suggestions, especially in readmes etc, should be as minimally prescriptive as possible. Folks are going to be copying and pasting it, and anything we bake in that they may not need by default is potentially confusing to them. Especially in this case the suggestion would fail for most folks who don't already have esbuild in their npx cache.

Copy link
Member

Choose a reason for hiding this comment

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

So yes, let's make it broader, exactly like your updated suggestions.

Copy link
Contributor

Choose a reason for hiding this comment

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

Just pushed those changes. Appreciate the explanation, that makes a lot of sense!

```
Use:
```bash
node . exec --yes false -- esbuild
```


## Performance & Benchmarks

We've set up an automated [benchmark](https://github.com/npm/benchmarks) integration that will run against all Pull Requests; Posting back a comment with the results of the run.
Expand Down