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>
7 participants
In a few places the code was refactored to use
maybeCallbackwhich always returns a function. Checking forif (callback)always returns true anyway.