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: remove empty VCLibrarianTool entry #17191

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
8 participants
@danbev
Member

danbev commented Nov 21, 2017

I've come across this empty entry a few times and tried building and running the tests on windows successfully. Wanted to bring this is up in case it was just overlooked.

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

build, win

cc @nodejs/build @nodejs/platform-windows

@refack

This comment has been minimized.

Show comment
Hide comment
@refack

refack Nov 21, 2017

Member

I always assumed it was left there as a hint that configuring LIB is possible via GYP (since it's not mentioned in the docs) 🤷‍♂️

Member

refack commented Nov 21, 2017

I always assumed it was left there as a hint that configuring LIB is possible via GYP (since it's not mentioned in the docs) 🤷‍♂️

@mscdex mscdex added the windows label Nov 21, 2017

@bzoz

This comment has been minimized.

Show comment
Hide comment
Contributor

bzoz commented Nov 23, 2017

@gibfahn

This comment has been minimized.

Show comment
Hide comment
@gibfahn

gibfahn Nov 23, 2017

Member

I always assumed it was left there as a hint that configuring LIB is possible via GYP (since it's not mentioned in the docs) 🤷‍♂️

Maybe replace with a comment that says that explicitly?

Member

gibfahn commented Nov 23, 2017

I always assumed it was left there as a hint that configuring LIB is possible via GYP (since it's not mentioned in the docs) 🤷‍♂️

Maybe replace with a comment that says that explicitly?

@bzoz

bzoz approved these changes Nov 23, 2017

@danbev

This comment has been minimized.

Show comment
Hide comment
@danbev

danbev Nov 23, 2017

Member

I always assumed it was left there as a hint that configuring LIB is possible via GYP (since it's not mentioned in the docs)

There is another usage here and I was thinking that it would be enough as reference.

Member

danbev commented Nov 23, 2017

I always assumed it was left there as a hint that configuring LIB is possible via GYP (since it's not mentioned in the docs)

There is another usage here and I was thinking that it would be enough as reference.

@refack

refack approved these changes Nov 25, 2017

@danbev

This comment has been minimized.

Show comment
Hide comment
@danbev

danbev Nov 27, 2017

Member

Landed in e308142

Member

danbev commented Nov 27, 2017

Landed in e308142

@danbev danbev closed this Nov 27, 2017

danbev added a commit that referenced this pull request Nov 27, 2017

build: remove empty VCLibrarianTool entry
PR-URL: #17191
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

@danbev danbev deleted the danbev:remove-empty-vslibrariantool branch Nov 27, 2017

@addaleax addaleax removed the author ready label Nov 28, 2017

MylesBorins added a commit that referenced this pull request Dec 12, 2017

build: remove empty VCLibrarianTool entry
PR-URL: #17191
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

MylesBorins added a commit that referenced this pull request Dec 12, 2017

build: remove empty VCLibrarianTool entry
PR-URL: #17191
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

MylesBorins added a commit that referenced this pull request Dec 12, 2017

build: remove empty VCLibrarianTool entry
PR-URL: #17191
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

@MylesBorins MylesBorins referenced this pull request Dec 12, 2017

Merged

v9.3.0 proposal #17631

gibfahn added a commit that referenced this pull request Dec 19, 2017

build: remove empty VCLibrarianTool entry
PR-URL: #17191
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

gibfahn added a commit that referenced this pull request Dec 19, 2017

build: remove empty VCLibrarianTool entry
PR-URL: #17191
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

@gibfahn gibfahn referenced this pull request Dec 20, 2017

Closed

v8.9.4 proposal #17772

gibfahn added a commit that referenced this pull request Dec 20, 2017

build: remove empty VCLibrarianTool entry
PR-URL: #17191
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

@gibfahn gibfahn referenced this pull request Dec 20, 2017

Merged

v8.9.4 proposal #17774

@MylesBorins MylesBorins referenced this pull request Dec 20, 2017

Merged

v6.12.3 proposal #17776

msoechting added a commit to hpicgs/node that referenced this pull request Feb 5, 2018

build: remove empty VCLibrarianTool entry
PR-URL: nodejs#17191
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>

msoechting added a commit to hpicgs/node that referenced this pull request Feb 7, 2018

build: remove empty VCLibrarianTool entry
PR-URL: nodejs#17191
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Gibson Fahnestock <gibfahn@gmail.com>
Reviewed-By: Bartosz Sosnowski <bartosz@janeasystems.com>
Reviewed-By: Refael Ackermann <refack@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment