tools: disallow extra blank lines at end of files by Trott · Pull Request #8920 · nodejs/node

Trott · 2016-10-04T03:46:37Z

Checklist

make -j8 test (UNIX), or vcbuild test nosign (Windows) passes
commit message follows commit guidelines

Affected core subsystem(s)

tools test

Description of change

In preparation for a lint rule that disallows empty lines at the end of a file, remove such lines from a number of test files. Refs: nodejs#8918

Fixes: nodejs#8918

Trott · 2016-10-04T03:49:33Z

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

targos

LGTM

mscdex · 2016-10-04T05:11:17Z

LGTM, CI failures are due to flaky tests/CI infrastructure issues.

lpinca · 2016-10-04T07:01:10Z

  new-parens: 2
  no-mixed-spaces-and-tabs: 2
-  no-multiple-empty-lines: [2, {max: 2}]
+  no-multiple-empty-lines: [2, {max: 2, maxEOF: 0}]


What about also adding maxBOF: 0 or is it better to do this in another pr?

What about also adding maxBOF: 0 or is it better to do this in another pr?

Sure, done, rebased, force-pushed.

lpinca

LGTM

addaleax · 2016-10-04T08:17:26Z

Do we check that files end in \n? Should we?

lpinca · 2016-10-04T08:21:33Z

@addaleax yes we have eol-last.

Fishrock123

Yes please. Arguably this is a bug of sorts in git, but alas.

That being said, this doesn't just happen on JS files.

Trott · 2016-10-04T16:54:24Z

Updated to include maxBOF rule too. Running the linter in CI again: https://ci.nodejs.org/job/node-test-linter/4632/

imyller

LGTM

ChALkeR

LGTM

targos

LGTM

addaleax

LGTM

In preparation for a lint rule that disallows empty lines at the end of a file, remove such lines from a number of test files. Refs: nodejs#8918 PR-URL: nodejs#8920 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Enabling linting to disallow extra blank lines at the start or end of JavaScript files in our code base. Fixes: nodejs#8918 PR-URL: nodejs#8920 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Trott · 2016-10-06T04:13:13Z

Landed in 5b2a805 and 5e6ae83

In preparation for a lint rule that disallows empty lines at the end of a file, remove such lines from a number of test files. Refs: #8918 PR-URL: #8920 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Enabling linting to disallow extra blank lines at the start or end of JavaScript files in our code base. Fixes: #8918 PR-URL: #8920 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

In preparation for a lint rule that disallows empty lines at the end of a file, remove such lines from a number of test files. Refs: #8918 PR-URL: #8920 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Enabling linting to disallow extra blank lines at the start or end of JavaScript files in our code base. Fixes: #8918 PR-URL: #8920 Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Brian White <mscdex@mscdex.net> Reviewed-By: Luigi Pinca <luigipinca@gmail.com> Reviewed-By: Ilkka Myller <ilkka.myller@nodefield.com> Reviewed-By: Сковорода Никита Андреевич <chalkerx@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Roman Reiss <me@silverwind.io> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Jeremiah Senkpiel <fishrock123@rocketmail.com> Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com> Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>

Trott added 2 commits October 3, 2016 20:43

test: remove blank lines at end of files

faced77

In preparation for a lint rule that disallows empty lines at the end of a file, remove such lines from a number of test files. Refs: nodejs#8918

tools: disallow extra blank lines at end of files

77fefaf

Fixes: nodejs#8918

Trott added the tools Issues and PRs related to the tools directory. label Oct 4, 2016

Trott mentioned this pull request Oct 4, 2016

Code style: linting whitespace at the end of the file. #8918

Closed

targos approved these changes Oct 4, 2016

View reviewed changes

bnoordhuis approved these changes Oct 4, 2016

View reviewed changes

lpinca reviewed Oct 4, 2016

View reviewed changes

lpinca approved these changes Oct 4, 2016

View reviewed changes

gibfahn approved these changes Oct 4, 2016

View reviewed changes

Fishrock123 approved these changes Oct 4, 2016

View reviewed changes

mhdawson approved these changes Oct 4, 2016

View reviewed changes

Trott force-pushed the empty-lines-eol branch from 77fefaf to c096c13 Compare October 4, 2016 16:52

imyller approved these changes Oct 4, 2016

View reviewed changes

ChALkeR approved these changes Oct 4, 2016

View reviewed changes

targos approved these changes Oct 4, 2016

View reviewed changes

silverwind approved these changes Oct 4, 2016

View reviewed changes

addaleax approved these changes Oct 5, 2016

View reviewed changes

Trott force-pushed the empty-lines-eol branch from c096c13 to 77fefaf Compare October 6, 2016 04:12

Trott closed this Oct 6, 2016

MylesBorins added the dont-land-on-v4.x label Oct 6, 2016

Trott deleted the empty-lines-eol branch January 13, 2022 22:44

Conversation

Trott commented Oct 4, 2016

Checklist

Affected core subsystem(s)

Description of change

Uh oh!

Trott commented Oct 4, 2016

Uh oh!

targos left a comment

Choose a reason for hiding this comment

Uh oh!

mscdex commented Oct 4, 2016

Uh oh!

lpinca Oct 4, 2016

Choose a reason for hiding this comment

Uh oh!

Trott Oct 4, 2016

Choose a reason for hiding this comment

Uh oh!

lpinca left a comment

Choose a reason for hiding this comment

Uh oh!

addaleax commented Oct 4, 2016

Uh oh!

lpinca commented Oct 4, 2016

Uh oh!

Fishrock123 left a comment

Choose a reason for hiding this comment

Uh oh!

Trott commented Oct 4, 2016

Uh oh!

imyller left a comment

Choose a reason for hiding this comment

Uh oh!

ChALkeR left a comment

Choose a reason for hiding this comment

Uh oh!

targos left a comment

Choose a reason for hiding this comment

Uh oh!

addaleax left a comment

Choose a reason for hiding this comment

Uh oh!

Trott commented Oct 6, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

13 participants