Remove unused branches from fs #4795
Conversation
|
LGTM if the CI is happy |
|
LGTM |
|
Any idea why the CI isn't happy? Doesn't seem related. |
|
Just Jenkins being Jenkins. I just got a green build though, so trying again: https://ci.nodejs.org/job/node-test-pull-request/1347/ |
|
LGTM |
In a few places the code was refactored to use `maybeCallback` which always returns a function. Checking for `if (callback)` always returns true anyway. PR-URL: nodejs#4795 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: thefourtheye <thechargingvolcano@gmail.com>
|
Ok, fixed commit message as far as I'm concerned this can land and it got 3 LGTMs. Thanks for the fast feedback as always :) |
|
Weird build bot failures happening in CI right now... just to be safe following the new commit, new CI run: https://ci.nodejs.org/job/node-test-pull-request/1351/ |
|
@jasnell good idea. Passed. |
In a few places the code was refactored to use `maybeCallback` which always returns a function. Checking for `if (callback)` always returns true anyway. PR-URL: #4795 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: thefourtheye <thechargingvolcano@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
|
Thanks! Landed in c00d08f. @benjamingr Thanks for pre-filling the review lines! I lowercased the commit title and wrapped the message body at 72 character, for reference. 😉 |
|
@silverwind thanks, I totally forgot about that - been a while since I've been able to contribute a PR :) |
In a few places the code was refactored to use `maybeCallback` which always returns a function. Checking for `if (callback)` always returns true anyway. PR-URL: #4795 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: thefourtheye <thechargingvolcano@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
|
@benjamingr it looks like this PR is modifying a bunch of code paths that don't exist on v4.x-staging and thus we are getting a ton of conflicts. Would you be willing to manually backport? |
|
@thealphanerd sure, do I just open a pull request against the v4 branch? |
|
against v4.x-staging |
In a few places the code was refactored to use `maybeCallback` which always returns a function. Checking for `if (callback)` always returns true anyway. PR-URL: nodejs#4795 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: thefourtheye <thechargingvolcano@gmail.com> Reviewed-By: Roman Reiss <me@silverwind.io>
In a few places the code was refactored to use
maybeCallbackwhich always returns a function. Checking forif (callback)always returns true anyway.