Backport remove unused branches from fs #5289
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
|
@benjamingr can you update the PR title to the original title? In fact the original PR Tittle / message would be useful to have |
|
and by PR I meant commit |
Before this commit, it was possible to push a partial character to a readable stream where it was decoded as an empty string and then added to the internal buffer. This caused the stream to not emit any data, even when the rest of the character bytes were pushed separately, because of a non-zero length check of the first chunk in the internal buffer. Fixes: nodejs#5223 PR-URL: nodejs#5226 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]>
Put links in a lexical order. Add missing links. Remove duplicates. PR-URL: nodejs#5072 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]>
Clarifies the code of conduct by making the following changes: - Adds section headings to make it easier to quickly parse. - Adds easy to find contact information. - Adds link to TSC moderation policies. - Moves attribution to the bottom of the page. PR-URL: nodejs#5107 Reviewed-By: Myles Borins <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Kat Marchán <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]>
Fix several typos in comments. PR-URL: nodejs#5279 Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#4767 Refs: nodejs/docs#38 Reviewed-By: Bryan English <[email protected]> Reviewed-By: Stephan Belanger <[email protected]>
Approved at CTC meeting nodejs#5409 Reviewers are CTC members who voted for this. PR-URL: nodejs#5278 Reviewed-By: Brian White <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Chris Dickinson <[email protected]>
Approved at CTC meeting nodejs#5409 Reviewers are CTC members who voted for this. PR-URL: nodejs#5277 Reviewed-By: Brian White <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Chris Dickinson <[email protected]>
Approved at CTC meeting nodejs#5409 Reviewers are CTC members who voted for this. PR-URL: nodejs#5276 Reviewed-By: Brian White <[email protected]> Reviewed-By: Trevor Norris <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]> Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Jeremiah Senkpiel <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Alexis Campailla <[email protected]> Reviewed-By: Chris Dickinson <[email protected]>
The APIs are implemented but currently not documented. PR-URL: nodejs#5402 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Rod Vagg <[email protected]>
https://code.google.com/p/v8/ redirects to the V8 issue tracker PR-URL: nodejs#5530 Reviewed-By: Ben Noordhuis <[email protected]>
Invoke MSBuild specifying the target platform as generated by Gyp. Reviewed-By: James M Snell <[email protected]> PR-URL: nodejs#5627
Array#pop() is known to be faster than Array#shift(). To be exact, it's O(1) vs. O(n). In this case there's no difference from which side of the "pool" array the object is retrieved, so .pop() should be preferred. PR-URL: nodejs#2174 Reviewed-By: mscdex - Brian White <[email protected]> Reviewed-By: jasnell - James M Snell <[email protected]> Reviewed-By: ofrobots - Ali Ijaz Sheikh <[email protected]>
PR-URL: nodejs#3726 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Chris Dickinson <[email protected]> Reviewed-By: Rich Trott <[email protected]>
MylesBorins
force-pushed
the
v4.x-staging
branch
from
a300696 to
cbb9633
Compare
The docs mentioned that the docs source live in the node source, but did not link to same. PR-URL: nodejs#4591 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: James M Snell <[email protected]>
This changes vcbuild.bat to accept a new parameter (vc2015 or vc2013) to select the version of Visual Studio to use. PR-URL: nodejs#4645 Reviewed-By: João Reis <[email protected]>
AfterGetAddrInfo() can potentially return an empty array of results without setting an error value. The JavaScript layer expects the array to have at least one value if an error is not returned. This commit sets a UV_EAI_NODATA error when an empty result array is detected. Fixes: nodejs#4545 PR-URL: nodejs#4715 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Evan Lucas <[email protected]> Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
ReadableState has the resumeScheduled property that helps determine if a stream should be resumed. It was not assigned in the constructor. When stream.resume is called on a readable stream that is not flowing, it is set to true. This changes the property map of the ReadableState which can cause a deopt in onEofChunk and needMoreData. PR-URL: nodejs#4761 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Brian White <[email protected]>
Two cluster tests have recently changed so that they are no longer resource intensive. Move them back to parallel. Ref: nodejs#4736 Ref: nodejs#4739 PR-URL: nodejs#4774 Reviewed-By: Johan Bergström <[email protected]> Reviewed-By: Colin Ihrig <[email protected]>
The current documentation for writable.write only specifies that the callback is called "once the data has been fully handled". It is ambiguous whether this means "successfully handled" and, if so, whether the callback is called if the data can not be successfully handled (i.e. an error occurs). The ambiguity is not only in the documentation. The stream class implementations differ on this point. stream.Writable invokes the callback with any errors that occur during parameter checking or during calls to _write. However, not all classes return all errors to _write. zlib.Zlib does pass argument and state errors to the _write (_transform) callback, but does not pass data errors. http.OutgoingMessage passes argument type errors and some other types of errors, but not all. This inconsistency is behind issue nodejs#1746 and, I suspect, other issues in client code which passes a callback to write. This commit takes no position on whether the callback error behavior should changed, but simply attempts to document the current behavior in a way that is open to changes so that users are not caught by surprise. PR-URL: nodejs#4810 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Jeremy Whitlock <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]>
Add fromArrayLike() to handle logic of copying in values from array-like argument. PR-URL: nodejs#4948 Reviewed-By: Sakthipriyan Vairamani <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Trevor Norris <[email protected]>
As mentioned in the comment of the changed file, "a libuv limitation makes it necessary to bind()". But, that is not the case in this test. The subsequent call to send() results in an implicit bind(). PR-URL: nodejs#5023 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Saúl Ibarra Corretgé <[email protected]>
TransformState has the writeencoding property that gets set on the first _write. It is not declared when the transform state is initially constructed and can cause a deopt. PR-URL: nodejs#5032 Reviewed-By: Brian White <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]>
This commit reverts the const usage introduced by 68a6abc because v8 currently cannot optimize functions that contain these uses of const (unsupported phi use of const variable). The performance difference in this case can be up to ~130% for non-ascii/binary string encodings. PR-URL: nodejs#5134 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: James M Snell <[email protected]>
PR-URL: nodejs#5117 Reviewed-By: Roman Reiss <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Minwoo Jung <[email protected]>
`readable.setEncoding(null)` - may be the most preferable way to proxy a binary data without any encoding/decoding overhead PR-URL: nodejs#5155 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Roman Reiss <[email protected]>
Includes two patches for AIX. Adds support for both 32-bit and 64-bit files[0] and uses -RPf for cp instead of -af (which is unsupported)[1] [0] https://codereview.chromium.org/1319663007 [1] https://codereview.chromium.org/1368133002 PR-URL: nodejs#3487 Reviewed-By: Ben Noordhuis <[email protected]> Reviewed-By: Shigeki Ohtsu <[email protected]>
This issue has already submitted to the upstream in https://code.google.com/p/gyp/issues/detail?id=477 Use this commit until the upstream is to be fixed. PR-URL: nodejs#1325 Reviewed-By: Fedor Indutny <[email protected]> Reviewed-By: Ben Noordhuis <[email protected]>
PR-URL: nodejs#5250 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Sakthipriyan Vairamani <[email protected]>
The need for an overview of blocking vs non-blocking was identified in the docs WG Q1 roadmap. As there are several topics also pending creation, this one tries to hit the correct level of detail based on completion of the others. One which is referenced is https://github.com/nodejs/node/pull/4936/files and URLs within this PR need to change based on where that will land on the node website. PR-URL: nodejs#5326 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: Stephen Belanger <[email protected]>
Avoid putting github templates in the source tarballs. PR-URL: nodejs#5612 Reviewed-By: Rod Vagg <[email protected]> Reviewed-By: Rich Trott <[email protected]>
OPENSSL_NO_SSL2 and OPENSSL_NO_WEAK_SSL_CIPHERS are defined in opensslconf.h Fixes: nodejs/Release#85 PR-URL: nodejs#5630 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Fedor Indutny <[email protected]>
The string template was closed after `${buf.length}`
causing a syntax error within the example.
PR-URL: nodejs#5781
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: James M Snell <[email protected]>
|
@benjamingr ... can you rebase this and update? |
|
@benjamingr sorry this fell through the cracks In future if you add the |
|
I'll close this and make a new PR manually backporting it. What do I make the PR against and from? |
|
@benjamingr it is still against the right branch. (v4.x-staging) You could just force push over and maintain the history in this Pr |
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#5289 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: thefourtheye <[email protected]>
|
@thealphanerd done, started CI at https://ci.nodejs.org/job/node-test-pull-request/2017/console , not sure how to land backport commits into the v4 branch - is it like landing regular PRs except I push into |
|
Ci is green @thealphanerd merge at will |
MylesBorins
force-pushed
the
v4.x-staging
branch
from
825db16 to
ab5ad06
Compare
|
@benjamingr can you rebase against v4.x-staging |
MylesBorins
force-pushed
the
v4.x-staging
branch
from
df3a543 to
7384140
Compare
|
ping @benjamingr |
MylesBorins
pushed a commit
that referenced
this pull request
Apr 8, 2016
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: #5289 Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: thefourtheye <[email protected]>
|
landed in 56dda6f |
|
Whoops, sorry! |
|
No prob... I should have just landed it earlier, there were no conflict 😄 |
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Manually backport #4795
Requested by @thealphanerd in #4795 (comment)