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.

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

tools: disallow extra blank lines at end of files #8920

Closed
wants to merge 2 commits into
base: master
from

Conversation

@Trott
Member

Trott commented Oct 4, 2016

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

Fixes: #8918

Trott added some commits Oct 4, 2016

test: remove blank lines at end of files
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
@Trott

This comment has been minimized.

Show comment
Hide comment
@targos

targos approved these changes Oct 4, 2016

LGTM

@mscdex

This comment has been minimized.

Show comment
Hide comment
@mscdex

mscdex Oct 4, 2016

Contributor

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

Contributor

mscdex commented Oct 4, 2016

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

@@ -80,7 +80,7 @@ rules:
max-len: [2, 80, 2]
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}]

This comment has been minimized.

@lpinca

lpinca Oct 4, 2016

Member

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

@lpinca

lpinca Oct 4, 2016

Member

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

This comment has been minimized.

@Trott

Trott Oct 4, 2016

Member

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

Sure, done, rebased, force-pushed.

@Trott

Trott Oct 4, 2016

Member

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

Sure, done, rebased, force-pushed.

@lpinca

lpinca approved these changes Oct 4, 2016

LGTM

@addaleax

This comment has been minimized.

Show comment
Hide comment
@addaleax

addaleax Oct 4, 2016

Member

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

Member

addaleax commented Oct 4, 2016

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

@lpinca

This comment has been minimized.

Show comment
Hide comment
@lpinca

lpinca Oct 4, 2016

Member

@addaleax yes we have eol-last.

Member

lpinca commented Oct 4, 2016

@addaleax yes we have eol-last.

@gibfahn

gibfahn approved these changes Oct 4, 2016

@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

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Oct 4, 2016

Member

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

Member

Trott commented Oct 4, 2016

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

@imyller

imyller approved these changes Oct 4, 2016

LGTM

@ChALkeR

ChALkeR approved these changes Oct 4, 2016

LGTM

@targos

targos approved these changes Oct 4, 2016

LGTM

@addaleax

LGTM

Trott added a commit to Trott/io.js that referenced this pull request Oct 6, 2016

test: remove blank lines at end of files
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>

Trott added a commit to Trott/io.js that referenced this pull request Oct 6, 2016

tools: disallow extra blank lines at EOF/BOF
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

This comment has been minimized.

Show comment
Hide comment
@Trott

Trott Oct 6, 2016

Member

Landed in 5b2a805 and 5e6ae83

Member

Trott commented Oct 6, 2016

Landed in 5b2a805 and 5e6ae83

@Trott Trott closed this Oct 6, 2016

jasnell added a commit that referenced this pull request Oct 6, 2016

test: remove blank lines at end of files
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>

jasnell added a commit that referenced this pull request Oct 6, 2016

tools: disallow extra blank lines at EOF/BOF
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>

Fishrock123 added a commit that referenced this pull request Oct 11, 2016

test: remove blank lines at end of files
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>

Fishrock123 added a commit that referenced this pull request Oct 11, 2016

tools: disallow extra blank lines at EOF/BOF
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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment