GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — build: fix line length off by one error by BridgeAR · Pull Request #24748 · nodejs/node

Conversation

@BridgeAR

@BridgeAR BridgeAR commented Nov 30, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
antsmartian reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@BridgeAR
build: fix line length off by one error
1753aa9 
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 commented Nov 30, 2018

Copy link
Copy Markdown
Collaborator

@BridgeAR build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/1742/pipeline
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot nodejs-github-bot added test Issues and PRs related to Node.js core tests and test infrastructure. tools Issues and PRs related to the tools directory. labels Nov 30, 2018
addaleax
addaleax approved these changes Nov 30, 2018
View reviewed changes

@addaleax addaleax left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this 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…

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@targos

targos commented Nov 30, 2018

Copy link
Copy Markdown
Member

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@BridgeAR

BridgeAR commented Nov 30, 2018

Copy link
Copy Markdown
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.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@BridgeAR

BridgeAR commented Nov 30, 2018

Copy link
Copy Markdown
Member Author

#24486 caused it.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@BridgeAR
build: add line break as soon as the tests are done
0b56ab0
@BridgeAR

BridgeAR commented Nov 30, 2018

Copy link
Copy Markdown
Member Author

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
targos
targos approved these changes Nov 30, 2018
View reviewed changes
@targos

targos commented Nov 30, 2018

Copy link
Copy Markdown
Member

@nodejs/python
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@BridgeAR

BridgeAR commented Nov 30, 2018

Copy link
Copy Markdown
Member Author

CI https://ci.nodejs.org/job/node-test-pull-request/19083/
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@BridgeAR BridgeAR added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Nov 30, 2018
@danbev

danbev commented Dec 3, 2018 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@danbev

danbev commented Dec 3, 2018

Copy link
Copy Markdown
Contributor

Landed in 5b90902, and 3099647.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@danbev danbev closed this Dec 3, 2018
danbev pushed a commit that referenced this pull request Dec 3, 2018
@BridgeAR @danbev
build: fix line length off by one error
3099647 
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
@BridgeAR @danbev
build: add line break as soon tests are done
5b90902 
PR-URL: #24748
Refs: #24486
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
@antsmartian

antsmartian commented Dec 3, 2018

Copy link
Copy Markdown
Contributor

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
BridgeAR added a commit that referenced this pull request Dec 5, 2018
@BridgeAR
build: fix line length off by one error
e0e15da 
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
@BridgeAR
build: add line break as soon tests are done
6b71099 
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
v11.4.0 proposal #24854
Merged
4 tasks done
This was referenced Dec 7, 2018
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#261
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#262
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#263
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#264
Closed
2018-12-07 Version 11.4.0 (Current) @BridgeAR beevelop/docker-android-nodejs#265
Closed
refack pushed a commit to refack/node that referenced this pull request Jan 14, 2019
@BridgeAR @refack
build: fix line length off by one error
6b3b868 
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
@BridgeAR @refack
build: add line break as soon tests are done
87881b4 
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
@BridgeAR @BethGriggs
build: add line break as soon tests are done
18d81c9 
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
@BridgeAR @BethGriggs
build: fix line length off by one error
b72bc11 
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 added the land-on-v10.x label Feb 12, 2019
@BethGriggs BethGriggs mentioned this pull request Feb 12, 2019
v10.15.3 proposal #26063
Merged
rvagg pushed a commit that referenced this pull request Feb 28, 2019
@BridgeAR @rvagg
build: add line break as soon tests are done
5844a06 
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
@BridgeAR @rvagg
build: fix line length off by one error
84353ac 
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

Reviewers

@addaleax addaleax addaleax approved these changes
+1 more reviewer
@targos targos targos approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. test Issues and PRs related to Node.js core tests and test infrastructure. 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.

Uh oh!

There was an error while loading. Please reload this page.

7 participants

@BridgeAR @nodejs-github-bot @targos @danbev @antsmartian @addaleax @BethGriggs
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.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.