-
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: replace --diagnostic-report-* with --report-* #27312
Conversation
In the code base the word `report` is almost only used to refer to the diagnostic report when it's a noun, and it's programmable interface `process.report()` it not prefixed, so `report` should be unambiguous enough to use without `diagnostic`.
cc @nodejs/diagnostics |
I wonder if for consistency this might be a good time to change a couple of the flag suffixes to something shorter like in #27306, i.e. |
@mscdex I think path.format({dir: 'test', 'name': 'report.json'}); // test/report.json on UNIX
path.parse('test/report.json');
// { root: '',
// dir: 'test',
// base: 'report.json',
// ext: '.json',
// name: 'report' } And, the API of process.report() still takes |
Co-Authored-By: joyeecheung <joyeec9h3@gmail.com>
Landed in 94adfe9 |
In the code base the word `report` is almost only used to refer to the diagnostic report when it's a noun, and it's programmable interface `process.report()` it not prefixed, so `report` should be unambiguous enough to use without `diagnostic`. PR-URL: #27312 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
In the code base the word
report
is almost only used to refer tothe diagnostic report when it's a noun, and its programmable
interface
process.report()
is not prefixed, soreport
should beunambiguous enough to use without
diagnostic
.Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes