-
Notifications
You must be signed in to change notification settings - Fork 30k
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
fs: remove unused assignment #35642
fs: remove unused assignment #35642
Conversation
@iansu @bcoe @cjihrig This can totally wait until after the push to get everything set up for 15.0 and so on--I know there's a lot going on right now--but I was wondering about taking this trivial change a little further and removing the options validation entirely in cases like this when Another alternative is to keep the validation but warn the user when those other two options are set and Mentioning that in case I'm missing/forgetting something that makes that A Bad Idea. |
This comment has been minimized.
This comment has been minimized.
@Trott I think warning when setting either |
This comment has been minimized.
This comment has been minimized.
PR-URL: nodejs#35642 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
5626cef
to
7f25fe8
Compare
Landed in 7f25fe8 |
PR-URL: #35642 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com>
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes