worker: replace message types string by constants#21537
worker: replace message types string by constants#21537starkwang wants to merge 1 commit intonodejs:masterfrom
Conversation
lib/internal/worker.js
Outdated
There was a problem hiding this comment.
Changing these strings into numbers may have better performance when comparing message.type. Is it worth to do that ?
There was a problem hiding this comment.
What about our regular sort of constants kLoadScript kStdioWantsMoreData etc?
lib/internal/worker.js
Outdated
There was a problem hiding this comment.
I’d maybe add a _ between SERIALIZE and ERROR ;)
benjamingr
left a comment
There was a problem hiding this comment.
LGTM given the space between SERIALIZE_ERROR
9f6c5d6 to
91307e5
Compare
lib/internal/worker.js
Outdated
There was a problem hiding this comment.
I think the _ ended up in the wrong place? ;)
There was a problem hiding this comment.
Oops, my fault /w\
lib/internal/worker.js
Outdated
There was a problem hiding this comment.
What if we use variables instead of properties of an object? This way, mistakes/typos can be caught at linting time, and errors like the one above COULD_NOT_SERIALIZ_EERROR would be more difficult to make (one has to make it at least twice for it to pass linting).
There was a problem hiding this comment.
Using variables may not prevent typos like COULD_NOT_SERIALIZ_EERROR because most of developers are using auto complete when coding. Using properties of an object can make developer benefits from the auto complete (just type messageTypes. and these constants will be showed)
This change can prevent typos and redundant strings in code.
91307e5 to
c502795
Compare
|
Landed in ebf5b58 |
This change can prevent typos and redundant strings in code. PR-URL: #21537 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This change can prevent typos and redundant strings in code. PR-URL: #21537 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
This change can prevent typos and redundant strings in code.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes