Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[v6.x backport] tools: non-Ascii linter for /lib only #19493

Closed
wants to merge 26 commits into from
Closed

[v6.x backport] tools: non-Ascii linter for /lib only #19493

wants to merge 26 commits into from

Conversation

SirR4T
Copy link
Contributor

@SirR4T SirR4T commented Mar 20, 2018 •
edited
Loading

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

Original PR: #18043

@nodejs-github-bot nodejs-github-bot added build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory. labels Mar 20, 2018
@SirR4T SirR4T changed the base branch from master to v6.x-staging March 20, 2018 18:41
@SirR4T
Copy link
Contributor Author

SirR4T commented Mar 20, 2018

Waiting for make -j4 test to pass on localhost. Will update here once done.

@gibfahn
Copy link
Member

gibfahn commented Mar 26, 2018

CI: https://ci.nodejs.org/job/node-test-pull-request/13869/

tniessen and others added 21 commits March 28, 2018 12:21
@tniessen @MylesBorins
test: fix spelling in test case comments
1196355 
PR-URL: nodejs#18018
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Yuta Hiroto <hello@about-hiroppy.com>
@addaleax @MylesBorins
fs: fix createReadStream(…, {end: n}) for non-seekable fds
377fe93 
82bdf8f fixed an issue by silently modifying the `start`
option for the case when only `end` is passed, in order to perform
reads from a specified range in the file.

However, that approach does not work for non-seekable files, since
a numeric `start` option means that positioned reads will be used
to read data from the file.

This patch fixes that, and instead ends reading after a specified
size by adjusting the read buffer size.

This way we avoid re-introducing the bug that 82bdf8f fixed,
and align behaviour with the native file stream mechanism
introduced in nodejs#18936 as well.

Backport-PR-URL: nodejs#19411
PR-URL: nodejs#19329
Fixes: nodejs#19240
Refs: nodejs#18121
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chen Gang <gangc.cxy@foxmail.com>
@BethGriggs @MylesBorins
test: remove mark flaky for moved test
96c8781 
Remove 'flaky' in parallel.status for test-debug-signal-cluster as the
test was moved to sequential.

Refs: nodejs#13592
PR-URL: nodejs#19069
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
@tniessen @MylesBorins
crypto: reuse variable instead of reevaluation
1e1478a 
Backport-PR-URL: nodejs#19115
PR-URL: nodejs#17735
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@lance @MylesBorins
test: make common.mustNotCall show file:linenumber
7d0cdf2 
When a test fails via `common.mustNotCall` it is sometimes hard to
determine exactly what was called. This modification stores the
caller's file and line number by using the V8 Error API to capture
a stack at the time `common.mustNotCall()` is called. In the event
of failure, this information is printed.

This change also exposes a new function in test/common, `getCallSite()`
which accepts a `function` and returns a `String` with the file name and
line number for the function.

Backport-PR-URL: nodejs#19355
PR-URL: nodejs#17257
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Khaidi Chu <i@2333.moe>
@MylesBorins
test: remove n-api intermediate files
630a306 
PR-URL: nodejs#19375
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@kfarnung @MylesBorins
test: remove orphaned entries from status
bf88a08 
PR-URL: nodejs#19042
Reviewed-By: Myles Borins <myles.borins@gmail.com>
@lpinca @MylesBorins
http: free the parser before emitting 'upgrade'
94a954a 
Ensure that the parser is freed before emitting the 'connect' or
'upgrade' event.

PR-URL: nodejs#18209
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@MylesBorins
stream: simplify src._readableState to state
0f593be 
PR-URL: nodejs#18264
Reviewed-By: Weijia Wang <starkwang@126.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@ofrobots @MylesBorins
doc: reorder section on updating PR branch
1bc9c4e 
It makes more sense to provide instructions on how to update the PR
branch before instructions on pushing the commit.

PR-URL: nodejs#18355
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
@Trott @MylesBorins
doc: add Gibson Fahnestock to TSC
dd6bcc6 
Welcome Gibson to the TSC!

PR-URL: nodejs#18481
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@Trott @MylesBorins
doc: move Brian White to TSC Emeriti list
83e5f41 
PR-URL: nodejs#18482
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ali Ijaz Sheikh <ofrobots@google.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Evan Lucas <evanlucas@me.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
@Trott @MylesBorins
doc: streamline README intro
9c4f6f5 
Shorten text that is duplicated from website and supply link.

PR-URL: nodejs#18483
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@jvelezpo @MylesBorins
test: verify the shell option works properly on execFile
de312a3 
Useful for executing in a shell because it accepts arguments as
an array instead of a string as exec does.
Depending on the circumstances,
that can prove to be useful if the arguments are already prepared.

PR-URL: nodejs#18384
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@rvagg @MylesBorins
build: allow x86_64 as a dest_cpu alias for x64
67e9498 
x86_64 is a standard arch descriptor on Linux, allow it as an alias for
our preferred descriptor: x64

PR-URL: nodejs#18052
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Joyee Cheung <joyeec9h3@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@lpinca @MylesBorins
net: remove redundant code from _writeGeneric()
5dade66 
The encoding is already handled by `Writable.prototype.write()`.

PR-URL: nodejs#18429
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@tniessen @MylesBorins
test: update references to archived repository
ebd9a18 
Backport-PR-URL: nodejs#19120
PR-URL: nodejs#17924
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@tniessen @MylesBorins
readline: update references to archived repository
096e4c6 
Backport-PR-URL: nodejs#19120
PR-URL: nodejs#17924
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Jon Moss <me@jonathanmoss.me>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@yhwang @MylesBorins
build: refine static and shared lib build
ddcfffb 
Refine the static and shared lib build process in order
to integrate static and shared lib verfication into CI.
When building both static and shared lib, we still build
node executable now and it uses the shared and static lib.

Signed-off-by: Yihong Wang <yh.wang@ibm.com>

Refs: nodejs#14158
Backport-PR-URL: nodejs#19050
PR-URL: nodejs#17604
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Daniel Bevenius <daniel.bevenius@gmail.com>
@lpinca @MylesBorins
stream: remove unreachable code
eeaecce 
To avoid a function call `BufferList.prototype.concat()` is not called
when there is only a buffer in the list. That buffer is instead
accessed directly.

Backport-PR-URL: nodejs#19483
PR-URL: nodejs#18239
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
@lpinca @MylesBorins
test: use correct size in test-stream-buffer-list
e22a645 
The `n` argument of `BufferList.prototype.concat()` is not the number
of `Buffer` instances in the list, but their total length when
concatenated.

Backport-PR-URL: nodejs#19483
PR-URL: nodejs#18239
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
vsemozhetbyt and others added 4 commits March 28, 2018 12:21
@vsemozhetbyt @MylesBorins
doc: linkify missing types
4444802 
Also, alphabetize all types in type-parser.js
and fix some nits in type formats.

Backport-PR-URL: nodejs#19500
PR-URL: nodejs#18444
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
@bzoz @MylesBorins
win, build: fix without-intl option
a47fda4 
Fixes --with-intl option passed to configure script when without-intl
is used

Backport-PR-URL: nodejs#19485
PR-URL: nodejs#17614
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
@poiru @MylesBorins
win, build: fix intl-none option
d4d4cec 
Like nodejs#17614, but for the `intl-none` option.

Backport-PR-URL: nodejs#19485
Refs: nodejs#17614
PR-URL: nodejs#18292
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Gireesh Punathil <gpunathi@in.ibm.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
@addaleax @MylesBorins
process: fix reading zero-length env vars on win32
0a4c79b 
Up until now, Node did not clear the current error code
attempting to read environment variables on Windows.
Since checking the error code is the way we distinguish between
missing and zero-length environment variables, this could lead to a
false positive when the error code was still tainted.

In the simplest case, accessing a missing variable and then a
zero-length one would lead Node to believe that both calls yielded
an error.

Before:

    > process.env.I=''; process.env.Q; process.env.I
    undefined
    > process.env.I=''; /*process.env.Q;*/ process.env.I
    ''

After:

    > process.env.I=''; process.env.Q; process.env.I
    ''
    > process.env.I=''; /*process.env.Q;*/ process.env.I
    ''

This only affects Node 8 and above, since before
1aa595e we always constructed a
`v8::String::Value` instance for passing the lookup key to the OS,
which in in turn always made a heap allocation and therefore
reset the error code.

Backport-PR-URL: nodejs#19484
PR-URL: nodejs#18463
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com>
Reviewed-By: Fedor Indutny <fedor.indutny@gmail.com>
Reviewed-By: Tobias Nießen <tniessen@tnie.de>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Tiancheng "Timothy" Gu <timothygu99@gmail.com>
Reviewed-By: Sakthipriyan Vairamani <thechargingvolcano@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
@SirR4T
tools: non-Ascii linter for /lib only
bc22bf0 
Non-ASCII characters in /lib get compiled into the node binary,
and may bloat the binary size unnecessarily. A linter rule may
help prevent this.

PR-URL: nodejs#18043
Fixes: nodejs#11209
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Teddy Katz <teddy.katz@gmail.com>
@jasnell jasnell added the v6.x label Apr 14, 2018
@ryzokuken
Copy link
Contributor

@MylesBorins does this need to be landed at all? v6.x is in maintenance, so I think not.

@SirR4T
Copy link
Contributor Author

SirR4T commented Aug 10, 2018

@ryzokuken should I close this PR then?

@SirR4T
Copy link
Contributor Author

SirR4T commented Aug 29, 2018

closing this PR, as 6.x is in maintenance, and this is not really a critical fix.
Will revive, if necessary.

@SirR4T SirR4T closed this Aug 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@MylesBorins MylesBorins

Labels
build Issues and PRs related to build files or the CI. doc Issues and PRs related to the documentations. meta Issues and PRs related to the general management of the project. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.