Skip to content

doc: clarify --watch and --watch-path usage with --run #58136

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

juanibe
Copy link

@juanibe juanibe commented May 3, 2025

This PR adds a note under the --watch and --watch-path CLI options in cli.md to clarify that these flags require a file path and cannot be used with --run.

Fixes confusion where users might assume --watch-path or --watch work with --run, which currently results in a runtime error.

Helps address issue discussion here

@nodejs-github-bot nodejs-github-bot added cli Issues and PRs related to the Node.js command line interface. doc Issues and PRs related to the documentations. labels May 3, 2025
@juanarbol
Copy link
Member

Hi! Thanks for taking care of this.

The PR may need some extra edits. The implies of watch in watch-path is not being shown, and also, just keep in mind that the --run flag has priority; It can not be mixed with watch and its derivatives. Could you please reflect that into your PR?

Refs:
https://github.com/nodejs/node/blob/main/src/node.cc#L1067

@juanibe
Copy link
Author

juanibe commented May 3, 2025

Hi @juanarbol!, thanks for the observation
I added the missing detail that --watch-path implicitly enables --watch and simplified the wording to reduce redundancy (since the reader can now refer to --watch). Let me know if any further clarification or modification is needed

Copy link
Member

@juanarbol juanarbol left a comment

Choose a reason for hiding this comment

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

Just some more minor clarifications, I'm sorry I just forgot a couple details, but after this; I'm more than happy with this PR. Thanks again!

Copy link
Member

@juanarbol juanarbol left a comment

Choose a reason for hiding this comment

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

RLGTM

@juanarbol
Copy link
Member

Worth reading doc.

https://github.com/nodejs/node/blob/70863fb9dd4b70f1ba6141db4403e69eb7a9eeff/CONTRIBUTING.md :)

@juanibe juanibe changed the title docs: clarify --watch and --watch-path usage with --run doc: clarify --watch and --watch-path usage with --run May 3, 2025
Copy link
Member

@juanarbol juanarbol left a comment

Choose a reason for hiding this comment

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

lgtm

@juanarbol
Copy link
Member

Test failure is already reported.

Refs: #58127

@juanarbol juanarbol added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 3, 2025
@juanibe juanibe force-pushed the docs/watch-cli-usage branch from a0fca22 to b740f44 Compare May 4, 2025 14:33
@juanibe
Copy link
Author

juanibe commented May 4, 2025

Updated the first commit message accordingly by amending it

@juanarbol
Copy link
Member

Updated the first commit message accordingly by amending it

Thanks. You don't have to do it now, but if you intend to land a single commit, squash everything into a single commit.

@juanibe juanibe force-pushed the docs/watch-cli-usage branch 2 times, most recently from 90561a2 to c5881e6 Compare May 4, 2025 19:51
@juanibe
Copy link
Author

juanibe commented May 6, 2025

Updated the first commit message accordingly by amending it

Thanks. You don't have to do it now, but if you intend to land a single commit, squash everything into a single commit.

Thanks! I've squashed the commits and the PR should be ready now. Let me know if there's anything else needed from my side.

@juanarbol
Copy link
Member

Thanks! I've squashed the commits and the PR should be ready now. Let me know if there's anything else needed from my side.

I'm sorry, CI still not happy due to commit guidelines. Fix it, will approve and wait for more reviews :-)

Humble ping to @nodejs/documentation

Refs:
https://github.com/nodejs/node/actions/runs/14824640752/job/41770212057#step:6:15

@juanibe juanibe force-pushed the docs/watch-cli-usage branch from c5881e6 to e8bef23 Compare May 7, 2025 09:06
@juanibe
Copy link
Author

juanibe commented May 7, 2025

Yeah, missed that part 🙏.
Already amended it
Thanks!

@juanibe juanibe requested a review from bnb May 12, 2025 09:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. cli Issues and PRs related to the Node.js command line interface. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants