-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
src: replace array implementation #24614
Conversation
replace with C++ API. Refs: nodejs#24125
edited to strike this off; normal code changes not eligible for fast-tracking under normal conditions. |
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.
looks good, but once again, normal code changes really shouldn't be fast tracked
@devsnek @apapirovski thanks for the correction, it was un-intentional! I struck the suggestion off. btw, Is there another PR around where I did the same mistake? From your description I felt so but unable to find one. Please let me know. |
@gireeshpunathil it was another code and learn pr, i don't think it was you specifically though. apologies now that i see how that message could be interpreted. |
landed as 4d3ee75 thank you @kazupon for the contribution! Wish you great success with continued contribution to this project, if you are further interested please have a look at https://www.nodetodo.org/next-steps |
replace with C++ API. Refs: nodejs#24125 PR-URL: nodejs#24614 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gus Caplan <me@gus.host>
replace with C++ API.
Refs: #24125
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes