-
Notifications
You must be signed in to change notification settings - Fork 29.7k
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: replace finally with PromisePrototypeFinally #35995
Conversation
If you could change other |
Sorry for the force push, I following the commit guidelines now (I hope) |
@baruchiro Can you rebase to resolve the conflict please?
|
About initializing the workspace and running Where should I open a bug and/or PR for that? (PR will be here, of course... But should I open an issue first? Do I need to open the issue in the current repo, even if it not related to |
If you have a fix, PR is very welcome. Issue is not necessary if your PR describes the problem and the solution. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@benjamingr As first-time contributor I allow myself to ask "what now?" When this PR will be merged? |
@baruchiro Your PR is |
Landed in 36fbbe0...a8b9ede |
#35993 (comment) PR-URL: #35995 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
@aduh95 So it actually taked my commits into Interesting technique 😀 |
@baruchiro yes, the reasons are outlined here: https://github.com/nodejs/node/blob/master/doc/guides/collaborator-guide.md#landing-pull-requests But anyway, you are now officially a Node.js contributor, congrats 🎉 |
Hooray!! 🥇 But now I can't write that in my CV, because I don't know how to answer the question "What you did as a Node.js contributor?"... 😉😉 Never mind, I'm sure the next contribution I will understand more. I just need to find the next one. |
Feel free to ping me on Facebook for more contribution ideas :) |
#35993 (comment) PR-URL: #35995 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
#35993 (comment) PR-URL: #35995 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
#35993 (comment) PR-URL: #35995 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
#35993 (comment) PR-URL: #35995 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com>
See suggestion here- #35993 (comment)
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes