[REVIEW] Fix gtest pinned cmake version for build from source option [skip-ci] #3175
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
dantegd
added
3 - Ready for Review
|
Please update the changelog in order to start CI tests. View the gpuCI docs here. |
wphicks
reviewed
Nov 23, 2020
| @@ -199,7 +199,7 @@ if(BUILD_GTEST) | |||
| include(ExternalProject) | |||
| ExternalProject_Add(googletest | |||
| GIT_REPOSITORY https://github.com/google/googletest.git | |||
| GIT_TAG 6ce9b98f541b8bcd84c5c5b3483f29a933c4aefb | |||
| GIT_TAG release-1.10.0 | |||
There was a problem hiding this comment.
Any reason not to use the commit SHA rather than the tag? I ask because I've been burned in the past by other projects moving their tags. Don't suspect it would be an issue with gtest, but it seems safer.
There was a problem hiding this comment.
I 100% agree with you, I tend to hate tags. But in this case the nice thing is that the tag coincides with the conda package version (and hasn't changed in a year), so I was thinking of doing an exception for this one, what do you think?
wphicks
approved these changes
Nov 23, 2020
| @@ -199,7 +199,7 @@ if(BUILD_GTEST) | |||
| include(ExternalProject) | |||
| ExternalProject_Add(googletest | |||
| GIT_REPOSITORY https://github.com/google/googletest.git | |||
| GIT_TAG 6ce9b98f541b8bcd84c5c5b3483f29a933c4aefb | |||
| GIT_TAG release-1.10.0 | |||
wphicks
added
5 - Ready to Merge
JohnZed
approved these changes
Nov 24, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Note: CI does not build from source, so skip ci seems appropriate