-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
lib: .load .save add proper error message when no file passed #52225
lib: .load .save add proper error message when no file passed #52225
Conversation
This commit adds a proper error message using ERR_MISSING_ARGS('file') when a .save or .load REPL command is runned. This commit also adds test for both of this cases. Fixes: nodejs#52218 Signed-off-by: Thomas Mauran <[email protected]>
This comment was marked as outdated.
This comment was marked as outdated.
@thomas-mauran Thank you for your contribution! The changes look good to me, but the linters have flagged some style issues. Could you please address them? |
Signed-off-by: Thomas Mauran <[email protected]>
Hello @cola119 I applied the lint correction I think the test will pass now |
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
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Hello @cola119 and @mertcanaltin I'm not sure of what should I do for the failed test ? |
They are flaky tests, so I re-run the CI. |
still some didn't pass @cola119 idk if it's because of the flaky test ? my pr shouldn't affect them |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Retrying CI... |
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
nice test seems to have passed ! thank you @cola119 |
Landed in 3f5ff8d |
@thomas-mauran Thank you for your contribution! I apologize for some flaky tests that prolonged the time it took for our CI to pass. I'm looking forward to your continued contributions in the future! |
no problem at all thanks for taking the time ! |
This commit adds a proper error message using ERR_MISSING_ARGS('file') when a .save or .load REPL command is runned. This commit also adds test for both of this cases. Fixes: #52218 Signed-off-by: Thomas Mauran <[email protected]> PR-URL: #52225 Reviewed-By: Kohei Ueno <[email protected]>
This commit adds a proper error message using ERR_MISSING_ARGS('file') when a .save or .load REPL command is runned. This commit also adds test for both of this cases. Fixes: #52218 Signed-off-by: Thomas Mauran <[email protected]> PR-URL: #52225 Reviewed-By: Kohei Ueno <[email protected]>
This commit adds a proper error message using ERR_MISSING_ARGS('file') when a .save or .load REPL command is runned. This commit also adds test for both of this cases.
Fixes: #52218