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

v8: include right headers in torque output #143

Closed
wants to merge 1 commit into from

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