-
Notifications
You must be signed in to change notification settings - Fork 35
Update gtest used to v1.15.2 #344
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
Update gtest used to v1.15.2 #344
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #344 +/- ##
==========================================
+ Coverage 74.43% 74.57% +0.14%
==========================================
Files 8 8
Lines 3204 3214 +10
==========================================
+ Hits 2385 2397 +12
+ Misses 819 817 -2 |
8de0922
to
7c1b615
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!
Please squash on merge. |
6ae2342
to
c60d845
Compare
823c24c
to
54113ef
Compare
@vgvassilev This PR needs another review, as I had to take a different approach after your last review revealed an error in my previous approach. |
cmake/modules/GoogleTest.cmake
Outdated
# issues due to [[maybe_unused]] in gtest) | ||
set(EXTRA_GTEST_OPTS -DCMAKE_OSX_SYSROOT=${CMAKE_OSX_SYSROOT}) | ||
set(CMAKE_CXX_FLAGS_TEMP_COPY ${CMAKE_CXX_FLAGS}) | ||
string(REPLACE "-pedantic" "" CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS}") |
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 am wondering if we need to propagate CMAKE_CXX_COMPILER
to the CMAKE_ARGS
. Alternatively in CMAKE_ARGS
we can append -no-pedantic
to disable the default mode.
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.
ping.
@vgvassilev Rather than add |
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!
This PR will attempt to update the version of gtest we are using to version 1.15.2 .
Fixes #295