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

docs: Update npm-doctor.md #6800

Merged
merged 6 commits into from
Sep 18, 2023
Merged

docs: Update npm-doctor.md #6800

merged 6 commits into from
Sep 18, 2023

Conversation

siemhesda
Copy link
Contributor

Changing description under npm doctor command to make it clearer

@siemhesda siemhesda requested a review from a team as a code owner September 12, 2023 19:55
@wraithgar
Copy link
Member

You're gonna want to run npm snap to update the docs snapshots I believe

@wraithgar
Copy link
Member

Apparently not!

@wraithgar
Copy link
Member

wraithgar commented Sep 13, 2023

Oh now I remember. You need to change the description attribute in lib/commands/doctor.js and then run npm run build -w docs. You'll also want to run npm run snap to update the test snapshots for the help output for the cli itself.

@siemhesda
Copy link
Contributor Author

Hi @wraithgar,
Check it out once more?

@wraithgar
Copy link
Member

You still need to commit the results of npm run snap

@siemhesda
Copy link
Contributor Author

Hi @wraithgar why are we tracking node_modules in the .gitignore line 30

@wraithgar
Copy link
Member

npm "vendors" its node_modules, but only production dependencies. The .gitignore reflects this and we have automated tooling to keep it up to date.

@siemhesda
Copy link
Contributor Author

I did a commit for the command

@wraithgar
Copy link
Member

Not sure why your changes aren't propagating when you update snapshots. Were there other files that didn't get committed?

@siemhesda
Copy link
Contributor Author

I also made a commit for npm run build -w doc that I had had not run before. How does it look now?

Copy link
Member

@wraithgar wraithgar left a comment

Choose a reason for hiding this comment

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

Yay looks like we got there!

@wraithgar wraithgar merged commit 8088325 into npm:latest Sep 18, 2023
24 of 25 checks passed
@github-actions github-actions bot mentioned this pull request Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants