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: add missing comma in node.gyp by cjihrig · Pull Request #31959 · nodejs/node

Conversation

@cjihrig

@cjihrig cjihrig commented Feb 26, 2020

Copy link
Copy Markdown
Contributor

This commit adds a missing comma for consistency with the surrounding lines.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
bricss reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@nodejs-github-bot nodejs-github-bot added the build Issues and PRs related to Node.js builds or CI infrastructure. label Feb 26, 2020
gengjiawen
gengjiawen approved these changes Feb 26, 2020
View reviewed changes
addaleax
addaleax approved these changes Feb 26, 2020
View reviewed changes
richardlau
richardlau approved these changes Feb 26, 2020
View reviewed changes
bnoordhuis
bnoordhuis approved these changes Feb 26, 2020
View reviewed changes

@bnoordhuis bnoordhuis 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.

Not just consistency. GYP works like python in that it concatenates strings unless separated by a comma.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
gengjiawen reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
lpinca
lpinca approved these changes Feb 26, 2020
View reviewed changes
@nodejs-github-bot

nodejs-github-bot commented Feb 27, 2020

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@nodejs-github-bot

nodejs-github-bot commented Feb 27, 2020 •
edited by cjihrig
Loading

Uh oh!

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

Copy link
Copy Markdown
Collaborator

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

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
ChALkeR
ChALkeR approved these changes Feb 27, 2020
View reviewed changes
@cjihrig cjihrig added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Feb 27, 2020
@gengjiawen

gengjiawen commented Feb 27, 2020 •
edited
Loading

Uh oh!

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

Copy link
Copy Markdown
Member

Can we fast track this ? Code navigation not working due to this.

Found MSVS version 15.0
configure  --dest-cpu=x64
Node.js configure: Found Python 2.7.17...
Warning: Missing input files:
src\large_pages\node_large_page.hsrc\memory_tracker.h
tools\msvs\genfiles\node_etw_provider.rc
tools\msvs\genfiles\node_etw_provider.h
All reactions

Sorry, something went wrong.

Uh oh!

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

cjihrig commented Feb 27, 2020

Copy link
Copy Markdown
Contributor Author

This has the approvals, CI run, etc. Please 👍 this comment to approve fast tracking as @gengjiawen requests.
gengjiawen and richardlau reacted with thumbs up emoji
All reactions
  • 👍 2 reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@cjihrig
build: add missing comma in node.gyp
a29b6cd 
This commit adds a missing comma for consistency with the
surrounding lines.

PR-URL: nodejs#31959
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
@cjihrig
cjihrig force-pushed the comma branch from 9a9f3f2 to a29b6cd Compare February 27, 2020 14:57
@cjihrig cjihrig added the fast-track PRs proposed for a shorter-than-standard waiting period before landing. label Feb 27, 2020
@cjihrig
cjihrig merged commit a29b6cd into nodejs:master Feb 27, 2020
@cjihrig
cjihrig deleted the comma branch February 27, 2020 14:58
codebytere pushed a commit that referenced this pull request Feb 27, 2020
@cjihrig @codebytere
build: add missing comma in node.gyp
ca44071 
This commit adds a missing comma for consistency with the
surrounding lines.

PR-URL: #31959
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
@codebytere codebytere mentioned this pull request Feb 29, 2020
v13.10.0 proposal #32027
Merged
@targos targos added the backport-blocked-v12.x label Apr 18, 2020
@targos

targos commented Apr 18, 2020

Copy link
Copy Markdown
Member

Depends on the large pages change to land on v12.x
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@targos targos removed author ready PRs with CI started, the required approvals, and no outstanding review comments. backport-blocked-v12.x labels Apr 25, 2020
targos pushed a commit to targos/node that referenced this pull request Apr 25, 2020
@cjihrig @targos
build: add missing comma in node.gyp
6c3897e 
This commit adds a missing comma for consistency with the
surrounding lines.

PR-URL: nodejs#31959
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
targos pushed a commit that referenced this pull request Apr 28, 2020
@cjihrig @targos
build: add missing comma in node.gyp
e765d59 
This commit adds a missing comma for consistency with the
surrounding lines.

PR-URL: #31959
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Сковорода Никита Андреевич <[email protected]>
@targos targos mentioned this pull request May 2, 2020
v12.17.0 release proposal #33197
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@bnoordhuis bnoordhuis bnoordhuis approved these changes
@ChALkeR ChALkeR ChALkeR approved these changes
@addaleax addaleax addaleax approved these changes
@lpinca lpinca lpinca approved these changes
@gengjiawen gengjiawen gengjiawen approved these changes
@richardlau richardlau richardlau approved these changes

Uh oh!

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

Assignees

No one assigned

Labels

build Issues and PRs related to Node.js builds or CI infrastructure. fast-track PRs proposed for a shorter-than-standard waiting period before landing.

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.

9 participants

@cjihrig @nodejs-github-bot @gengjiawen @targos @bnoordhuis @ChALkeR @addaleax @lpinca @richardlau
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.