This repository has been archived by the owner on Aug 31, 2018. It is now read-only.
Upstream update November 1 #127
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
PR-URL: nodejs/node#16259 Fixes: nodejs/node#16077 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
PR-URL: nodejs/node#15362 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Major V8 updates are usually API/ABI incompatible with previous versions. This commit adapts NODE_MODULE_VERSION for V8 6.2. Refs: https://github.com/nodejs/CTC/blob/master/meetings/2016-09-28.md PR-URL: nodejs/node#15362 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
With V8 6.2 there is one line less in the promise trace. PR-URL: nodejs/node#15362 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs/node#15362 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs/node#15362 Reviewed-By: Myles Borins <myles.borins@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Original commit message:
[build] Introduce an embedder version string
Sometimes, the embedder might want to merge a fix to an abandoned branch
or to a supported branch but the fix is not relevant to Chromium.
This adds a new version string that the embedder can set at compile time
and that will be appended to the official V8 version.
The separator must be provided in the string. For instance, to have a
full version string like "6.0.287.53-emb.1", the embedder must set
V8_EMBEDDER_STRING to "-emb.1".
Related Node.js issue: nodejs/node#9754
BUG=v8:5740
R=machenbach@chromium.org
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: Ifa2d9bd213795e6d54886436f8c3787ac6162823
Reviewed-on: https://chromium-review.googlesource.com/690475
Reviewed-by: Michael Achenbach <machenbach@chromium.org>
Reviewed-by: Yang Guo <yangguo@chromium.org>
Commit-Queue: Michaël Zasso <mic.besace@gmail.com>
Cr-Commit-Position: refs/heads/master@{#48301}
Refs: v8/v8@b096c44
PR-URL: nodejs/node#15785
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Original commit message:
[api] Add optional data pointer to GC callbacks
This can be useful when there may be multiple callbacks attached by
code that's not directly tied to a single isolate, e.g. working
on a per-context basis.
This also allows rephrasing the global non-isolate APIs in terms
of this new API, rather than working around it inside `src/heap`.
TBR=hpayer@chromium.org
Bug:
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_chromium_rel_ng
Change-Id: I2e490ec40d1a34ea812f25f41ef9741d2116d965
Reviewed-on: https://chromium-review.googlesource.com/647548
Reviewed-by: Yang Guo <yangguo@chromium.org>
Reviewed-by: Adam Klein <adamk@chromium.org>
Commit-Queue: Yang Guo <yangguo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#47923}
PR-URL: nodejs/node#15391
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
This change fixes a potential segfault in the sampling heap profiler. This landed as part of a larger change upstream [1]. This is the minimal backport that avoids the segfault. [1]: https://git.io/vdTYL PR-URL: nodejs/node#15498 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com>
Original commit message:
avoid constructor inheritance due to compilation issues
Constructor inheritance of a templated constructor is causing compilation issues for node.js:
https: //github.com/nodejs/node/pull/15362#issue-257007421
Change-Id: I7d099ff5a1a2fd5b19c11112ddef8fe824e509f7
Reviewed-on: https://chromium-review.googlesource.com/707008
Commit-Queue: Tobias Tebbi <tebbi@chromium.org>
Commit-Queue: Benedikt Meurer <bmeurer@chromium.org>
Reviewed-by: Benedikt Meurer <bmeurer@chromium.org>
Cr-Commit-Position: refs/heads/master@{#48445}
Refs: v8/v8@0f1dfae
PR-URL: nodejs/node#15362
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
PR-URL: nodejs/node#15970 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15840 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15872 Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15874 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15875 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15924 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15924 Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15938 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15959 Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
PR-URL: nodejs/node#15991 Reviewed-By: Yosuke Furukawa <yosuke.furukawa@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#15983 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
The `net`, `tls`, `http` and `https` module have the same `server.listen()` method, but have a different documenation. Changed to a consistent link to the documentation of the `net` module. PR-URL: nodejs/node#16020 Reviewed-By: Bryan English <bryan@bryanenglish.com> Reviewed-By: Sam Roberts <vieuxtech@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Fixes a line of uncovered code in the AsyncWrap class's constructor. Specifically this covers validtion of the 'promiseResolve' argument. PR-URL: nodejs/node#16025 Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
PR-URL: nodejs/node#16035 Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: James M Snell <jasnell@gmail.com>
PR-URL: nodejs/node#16040 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#15867 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
The 'Main' function in tools/test.py file was using a variable named ``all_outcomes`` to store a value not being used. It is a best practice to name unused variables, often return values of functions/methods (as in this case) as ``_`` [1]. This just helps keep the code a bit cleaner and avoid any silly mistakes. PR-URL: nodejs/node#16171 Refs: [1] https://stackoverflow.com/a/5477153 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Claudio Rodriguez <cjrodr@yahoo.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
PR-URL: nodejs/node#16219 Refs: nodejs/node#15362 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Evan Lucas <evanlucas@me.com> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Add an utility constructor for `AsyncWrap` classes that wish to leverage `InternalCallbackScope`s. PR-URL: nodejs/node#16461 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Sending pending data may involve running arbitrary JavaScript code. Therefore, it should involve a callback scope. PR-URL: nodejs/node#16461 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
When assigning to a union twice, the first assignment is a no-op. PR-URL: nodejs/node#16461 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
Use weak handles to track the lifetime of an `Http2Session` instance. Refs: nodejs/http2#140 PR-URL: nodejs/node#16461 Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Franziska Hinkelmann <franziska.hinkelmann@gmail.com>
The LICENSE file was not originally included when remark-cli was vendored. This Commit adds the LICENSE file back to tools/remark-cli and updates the top level license in the project PR-URL: nodejs/node#16637 Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs/node#16639 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: James M Snell <jasnell@gmail.com>
The original commit was landed without running CITGM. Unfortunately this change breaks the module `d` which has over 500k downloads a day. It is worth mentioning that the compatibility hack can be removed without breaking anything. We should definitely revisit for the next Semver-Major but shipping this today will cause non trivial ecosystem breakages. Refs: nodejs/node#3384 PR-URL: nodejs/node#16634 Refs: nodejs/node#3384 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This LTS release comes with 87 commits. This includes 30 that are
updates to lib/ or src/, 20 that are test related, 13 that are doc
related, 19 which are build / tools related, and 4 commits which are
updates to dependencies.
Notable Changes:
* doc:
- add Gibson Fahnestock to Release team (Gibson Fahnestock)
nodejs/node#16620
* deps:
- update npm to 5.5.1 (Myles Borins)
nodejs/node#16509
* http2:
- The exposed http2 socket is no longer manipulatable
(Anatoli Papirovski)
nodejs/node#16330
* module:
- support custom paths to require.resolve() (cjihrig)
nodejs/node#16397
* util:
- util.TextEncoder and util.TextDecoder are no longer experimental.
There will no longer be a warning when they are used
(James M Snell)
nodejs/node#15743
PR-URL: nodejs/node#16630
When `throw undefined` or `throw null` is executed, the REPL crashes. This change does a check for `null|undefined` before accessing an error's properties to prevent crashing. Fixes: nodejs/node#16545 Fixes: nodejs/node#16607 PR-URL: nodejs/node#16574 Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Lance Ball <lball@redhat.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Improved stream documentation with an example of how to decode buffers to strings within a custom Writable. Fixes: nodejs/node#15369 PR-URL: nodejs/node#16403 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
* Older experimental APIs have been removed.
[[`d731369b1d`](nodejs/node@d731369b1d)]
[#14414](nodejs/node#14414)
* **Errors**
* Improvements have been made to `buffer` module error messages.
* The assignment of static error codes to Node.js error continues:
* `buffer`
* `child_process`
* `console`
* `crypto`
* `dns`
* `events`
* `fs`
* `http`
* `inspector`
* `net`
* `path`
* `process`
* `querystring`
* `readline`
* `repl`
* `streams`
* `string_decoder`
* `timers`
* `tls`
* `url`
* `util`
* `v8`
* `zlib`
* **Child Processes**
* Errors are emitted on process nextTick.
* **Domains**
* The long-deprecated `.dispose()` method has been removed.
* **fs**
* The `fs.ReadStream` and `fs.WriteStream` classes now use `destroy()`.
* `fs` module callbacks are now invoked with an undefined context.
* **HTTP/1**
* A 400 Bad Request response will now be sent when parsing fails.
* Socket timeout will be set when the socket connects.
* A bug causing the request `'error'` event to fire twice was fixed.
* HTTP clients may now use generic `Duplex` streams in addition to `net.Socket`.
* **Intl**
* The deprecated `Intl.v8BreakIterator` has been removed.
* **Modules**
* The `require.resolve()` method now supports using custom lookup paths.
* **OS**
* The `os.EOL` property is now read-only.
* **Timers**
* `setTimeout()` will emit a warning if the timeout is larger that the maximum
32-bit unsigned integer.
PR-URL: nodejs/node#16653 Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Vse Mozhet Byt <vsemozhetbyt@gmail.com>
PR-URL: nodejs/node#15895 Reviewed-By: Daijiro Wachi <daijiro.wachi@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
PR-URL: nodejs/node#16606 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: XadillaX <admin@xcoder.in> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
This commit adds a missing comma in the sources list. This effects at least window which produces the following warning where trace_event.h and src/util.h are concatenated: Warning: Missing input files: src\tracing\trace_event.hsrc\util.h PR-URL: nodejs/node#16613 Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Richard Lau <riclau@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Minwoo Jung <minwoo@nodesource.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Add test for ModuleMap set with ModuleJob but bad url. PR-URL: nodejs/node#16045 Reviewed-By: Stephen Belanger <admin@stephenbelanger.com> Reviewed-By: Bradley Farias <bradley.meck@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
CloudABI is a compact POSIX-like runtime that makes use of capability-based security. More details: https://github.com/NuxiNL/cloudlibc * src: Disable use of pwd.h, grp.h and get*uid() on CloudABI. As CloudABI is intended to run applications in cluster contexts (e.g., on Kubernetes), they are oblivious of UNIX credentials. Extend the existing preprocessor checks to disable any use of these interfaces, just like on Windows, Android, etc. * src: Explicitly include <netdb.h>. cares_wrap.cc calls into functions like getnameinfo() and getaddrinfo(). These functions tend to be available implicitly through <uv.h>, but we'd better still include this header explicitly. On CloudABI, we make use of a custom implementation of libuv that does not implicitly include header files like <netdb.h>. PR-URL: nodejs/node#16612 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Refael Ackermann <refack@gmail.com>
PR-URL: nodejs/node#16632 Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Anatoli Papirovski <apapirovski@mac.com> Reviewed-By: Tobias Nießen <tniessen@tnie.de> Reviewed-By: Khaidi Chu <i@2333.moe> Reviewed-By: Brian White <mscdex@mscdex.net>
aqrln
approved these changes
Nov 2, 2017
addaleax
approved these changes
Nov 2, 2017
|
Seems to have a bunch of linting failures locally. I'll try a nodejs build to see how it compares. https://gist.github.com/Qard/5d6932944602f2579843469c722f11ed |
|
There's some lint errors for lack of documentation of worker errors and there's some from our top-level md files. We can probably at least ignore the top-level ones for now. More concerning though is that almost all the http2 tests seem to fail on ayo, but not on node. Not sure why yet. |
|
@Qard the HTTP/2 issues should be fixed by #133 + this patch: diff --git a/src/node_http2.cc b/src/node_http2.cc
index 99b909616152..19cbb13bb529 100644
--- a/src/node_http2.cc
+++ b/src/node_http2.cc
@@ -107,6 +107,8 @@ Http2Session::~Http2Session() {
}
void Http2Session::Close() {
+ if (prep_ == nullptr)
+ return; // Avoid double close.
if (!object().IsEmpty())
ClearWrap(object());
persistent().Reset(); |
|
I went ahead and resolved conflicts and landed this + the patch above Otherwise we’ll never be able to catch up ;) Landed in fbd7c6e...45a1c8c |
addaleax
added a commit
that referenced
this pull request
Dec 7, 2017
Refs: #127 (comment) PR-URL: #127
|
Thanks! 🎉 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This also includes everything in #124.
Skipped (README.md):
New upstream HEAD: