-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
Switch Travis to Xenial build #4789
Conversation
Is Travis failure related? |
c646a52
to
9017dde
Compare
@siying possibly a problem with CMake. I just rebased and pushed again, let's see what happens... |
1afacff
to
3d481a9
Compare
3d481a9
to
3bd5fc0
Compare
@@ -1,5 +1,9 @@ | |||
cmake_minimum_required(VERSION 3.4) | |||
|
|||
if(${CMAKE_VERSION} VERSION_LESS "3.11.4") | |||
message("Please consider switching to CMake 3.11.4 or newer") |
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.
You already error out in the relevant case below - is this warning helpful?
set(JNI_OUTPUT_DIR ${PROJECT_SOURCE_DIR}/java/include) | ||
file(MAKE_DIRECTORY ${JNI_OUTPUT_DIR}) | ||
|
||
if(${Java_VERSION_MAJOR} VERSION_GREATER_EQUAL "10" AND ${CMAKE_VERSION} VERSION_LESS "3.11.4") |
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.
i believe any version 3.11 or higher is sufficient
Let me land it now to cut the round trip time. Follow up comments can be addressed with follow up PR. |
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.
@siying is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
This pull request has been merged in d1ae67b. |
Summary: I think this should now also run on Travis's new virtualised infrastructure which affords more memory and CPU. We also need to think about migrating from travis-ci.org to travis-ci.com. Pull Request resolved: facebook#4789 Differential Revision: D15856272 fbshipit-source-id: 10b41d21924e8a362bc9646a63ccd1a5dfc437c6
I think this should now also run on Travis's new virtualised infrastructure which affords more memory and CPU.
We also need to think about migrating from travis-ci.org to travis-ci.com.