-
Notifications
You must be signed in to change notification settings - Fork 29.1k
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
src: remove VS 2013 compatibility hacks #8067
Conversation
more or less rubber stamp LGTM if CI is green where it should be |
There were some rather strange errors on the fedora22 and fedora23 builbots. New CI, just in case: https://ci.nodejs.org/job/node-test-pull-request/3628/ |
Working on the bits for not using VS2013 on v6 and after, already working for the compile job: https://ci.nodejs.org/job/node-test-pull-request/3669/ But the test job will have to run on machines with VS2013, because we still support building native modules with that. Will have to work more on that. It's great to have this work ready, but perhaps this should only land after we have the release servers ready, so that non-LTS releases of v6 don't have to work around this. |
That's fine, there is no rush to land this. |
#8412 is my suggestion to unblock this. After it lands, CI for this should be all green: test machines ( |
#8412 has landed, here is a passing CI: https://ci.nodejs.org/job/node-test-commit/4954/ Changes in |
62e8931
to
a0b6a4d
Compare
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.
LGTM
We can remove some Visual Studio 2013-specific workarounds now that support for that compiler has officially been dropped. PR-URL: nodejs#8067 Refs: nodejs#7484 Refs: nodejs#8049 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joao Reis <[email protected]>
Support for Visual Studio 2013 has officially been dropped, remove the build option for that compiler. PR-URL: nodejs#8067 Refs: nodejs#7484 Refs: nodejs#8049 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joao Reis <[email protected]>
a0b6a4d
to
a3f861d
Compare
We can remove some Visual Studio 2013-specific workarounds now that support for that compiler has officially been dropped. PR-URL: #8067 Refs: #7484 Refs: #8049 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joao Reis <[email protected]>
Support for Visual Studio 2013 has officially been dropped, remove the build option for that compiler. PR-URL: #8067 Refs: #7484 Refs: #8049 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joao Reis <[email protected]>
We can remove some Visual Studio 2013-specific workarounds now that support for that compiler has officially been dropped. PR-URL: #8067 Refs: #7484 Refs: #8049 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joao Reis <[email protected]>
Support for Visual Studio 2013 has officially been dropped, remove the build option for that compiler. PR-URL: #8067 Refs: #7484 Refs: #8049 Reviewed-By: James M Snell <[email protected]> Reviewed-By: Joao Reis <[email protected]>
We can remove some Visual Studio 2013-specific workarounds now that
support for that compiler has officially been dropped.
Refs: #7484
Refs: #8049
("has been dropped" == future tense.)
The last commit is something I can drop if people feel it's less readable.
CI: https://ci.nodejs.org/job/node-test-pull-request/3625/ (expected to fail on the vs2013 buildbots.)