Skip to content

Conversation

@gengjiawen
Copy link
Member

@gengjiawen gengjiawen commented Feb 27, 2020

Credits goes to @bnoordhuis , he does all the hard work. See #133.

I add one more line (4006).

    cc_contents << "#include \"src/objects/property-descriptor-object.h\"\n";

Verified on Visual Studio 2017 Developer Command Prompt v15.9.20.

Fix #128

@gengjiawen
Copy link
Member Author

Is it possible we drop support vs2017 ? We seemed spend way too much time on it.

@richardlau
Copy link
Member

Is it possible we drop support vs2017 ? We seemed spend way too much time on it.

I’d defer to @joaocgreis re. supported versions of VS.

@richardlau
Copy link
Member

Credits goes to @bnoordhuis , he does all the hard work. See #133.

Add a Co-authored-by to the commit message?

@gengjiawen
Copy link
Member Author

Credits goes to @bnoordhuis , he does all the hard work. See #133.

Add a Co-authored-by to the commit message?

Sure. I forced push the branch.

Co-authored-by: Ben Noordhuis <info@bnoordhuis.nl>
@gengjiawen gengjiawen force-pushed the canary branch 2 times, most recently from 12ced1e to 9a2dbec Compare February 27, 2020 14:38
@targos
Copy link
Member

targos commented Feb 27, 2020

Thanks.
So, I pushed it to canary-base in nodejs/node@a1066f0, but this should really be upstreamed at some point.

@joaocgreis
Copy link
Member

Is it possible we drop support vs2017 ? We seemed spend way too much time on it.

I think on Node.js 14 it is still a bit too premature, but I'd have no objections for 15 or 16.

To be clear, this is for building Node.js core. For building add-ons, we should support it as long as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Build broken on Windows (VS2017)

4 participants