process: internal/process/stdio.js cleanup / modernization#6766
Conversation
There was a problem hiding this comment.
is configurable: false, enumerable: true the default for __defineGetter__? i.e. this isn't breaking is it?
There was a problem hiding this comment.
ah, right, __defineGetter__ uses configurable: true. Will switch that.
|
+0, it's a bit churny and I'm unsure this buys us enough to warrant the churn and if we're going to churn then maybe we should make it even nicer by hoisting those functions out of there. |
|
hoisting them out and putting them where? |
9264bd3 to
56b5e2f
Compare
|
In the file, something seen often times as better practice. Object.defineProperty(process, 'stdout' { get: getStdout })
function getStdout(){}Edit: this doesn't seem to gain us anything though? +-0 |
|
If that's the "better practice" then I'd assume it's something we'd promote across core... a quick search shows that it's not (and hasn't been) done that way consistently. In any case, hoisted the things. |
|
Same comment. Churn, but the code changes themselves LGTM. |
|
CI is green except for an unrelated flaky test. |
Avoid using deprecated getter syntax plus other miscellaneous updates.
61a26e5 to
4a828a7
Compare
Avoid using deprecated getter syntax plus other miscellaneous updates. PR-URL: #6766 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
Landed in f856234 |
Avoid using deprecated getter syntax plus other miscellaneous updates. PR-URL: #6766 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
|
@jasnell set to don't land.. feel free to change that |
Avoid using deprecated getter syntax plus other miscellaneous updates. PR-URL: #6766 Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Checklist
Affected core subsystem(s)
process (internal)
Description of change
Avoid using deprecated getter syntax plus other miscellaneous updates.