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.

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

build: fix line length off by one error #24748

Closed
wants to merge 2 commits into from

Conversation

BridgeAR
Copy link
Member

@BridgeAR BridgeAR commented Nov 30, 2018

While running the test suite the progress bar shows former line
endings if the new line is shorter than the former line. The length
was calculated without the line ending. It is now an empty string
to prevent the off by one error instead of using extra whitespace.

Update: I also fixed the missing new line when finishing the
tests.

Refs: #24486

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

While running the test suite the progress bar shows former line
endings if the new line is shorter than the former line. The length
was calculated without the line ending. It is now an empty string
to prevent the off by one error instead of using extra whitespace.
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory. labels Nov 30, 2018
Copy link
Member

@addaleax addaleax left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know what caused this? It seems to be a recent phenomenon…

@targos
Copy link
Member

targos commented Nov 30, 2018

Same question as @addaleax. I thought it was a bug with my terminal...

@BridgeAR
Copy link
Member Author

I did not check but I'll try to figure out what regressed. There seems to be another issue with a missing new line when finishing the tests.

@BridgeAR
Copy link
Member Author

#24486 caused it.

@BridgeAR
Copy link
Member Author

I just pushed another commit to add the missing new line as well.

@targos
Copy link
Member

targos commented Nov 30, 2018

@nodejs/python

@BridgeAR
Copy link
Member Author

@BridgeAR BridgeAR added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Nov 30, 2018
@danbev
Copy link
Contributor

danbev commented Dec 3, 2018

Re-run of failing node-test-commit-linux ✔️

@danbev
Copy link
Contributor

danbev commented Dec 3, 2018

Landed in 5b90902, and 3099647.

@danbev danbev closed this Dec 3, 2018
danbev pushed a commit that referenced this pull request Dec 3, 2018
While running the test suite the progress bar shows former line
endings if the new line is shorter than the former line. The length
was calculated without the line ending. It is now an empty string
to prevent the off by one error instead of using extra whitespace.

PR-URL: #24748
Refs: #24486
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
danbev pushed a commit that referenced this pull request Dec 3, 2018
PR-URL: #24748
Refs: #24486
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@antsmartian
Copy link
Contributor

Thanks @BridgeAR, I thought my terminal had issues..

BridgeAR added a commit that referenced this pull request Dec 5, 2018
While running the test suite the progress bar shows former line
endings if the new line is shorter than the former line. The length
was calculated without the line ending. It is now an empty string
to prevent the off by one error instead of using extra whitespace.

PR-URL: #24748
Refs: #24486
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BridgeAR added a commit that referenced this pull request Dec 5, 2018
PR-URL: #24748
Refs: #24486
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@BridgeAR BridgeAR mentioned this pull request Dec 5, 2018
4 tasks
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
While running the test suite the progress bar shows former line
endings if the new line is shorter than the former line. The length
was calculated without the line ending. It is now an empty string
to prevent the off by one error instead of using extra whitespace.

PR-URL: nodejs#24748
Refs: nodejs#24486
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
PR-URL: nodejs#24748
Refs: nodejs#24486
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
PR-URL: #24748
Refs: #24486
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 12, 2019
While running the test suite the progress bar shows former line
endings if the new line is shorter than the former line. The length
was calculated without the line ending. It is now an empty string
to prevent the off by one error instead of using extra whitespace.

PR-URL: #24748
Refs: #24486
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@BethGriggs BethGriggs mentioned this pull request Feb 12, 2019
rvagg pushed a commit that referenced this pull request Feb 28, 2019
PR-URL: #24748
Refs: #24486
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
rvagg pushed a commit that referenced this pull request Feb 28, 2019
While running the test suite the progress bar shows former line
endings if the new line is shorter than the former line. The length
was calculated without the line ending. It is now an empty string
to prevent the off by one error instead of using extra whitespace.

PR-URL: #24748
Refs: #24486
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@BridgeAR BridgeAR deleted the fix-off-by-one-error branch January 20, 2020 11:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. test Issues and PRs related to the tests. tools Issues and PRs related to the tools directory.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants