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

rebase upstream master #1

Merged
merged 440 commits into from
Jan 18, 2018
Merged

rebase upstream master #1

merged 440 commits into from
Jan 18, 2018

Conversation

dirk-thomas
Copy link

This is necessary to compile without warnings in Visual Studio 2017. The rebase pulls in several fixes which have been applied.

Instead of merging the PR I would propose to force push the changes to the default branch. That keeps the history simpler, makes future rebases trivial, and makes it easier to see the custom commits applied by us.

The change doesn't affect the "normal" builds:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

gennadiycivil and others added 30 commits August 31, 2017 11:13
change links from former code.google.com to current github repository
…uild-type

use cmake build type defined in .travis.yml for travis builds
Wrong version reported (1.7.0 should be 1.8.0)
Performance fixes reported by cppcheck
If the user's cmakelists.txt first look for threads using
find_package(Threads), then set(gtest_disable_pthreads ON),
and then include googletest. GoogleTest will not look for
threads. But since they have already been found before in
user's cmakelists, it will use them regardless.

This helped me fix build issue in darktable-org/rawspeed
on windows/MSYS2, even though there are threads, and they
are usable, googletest build was failing with issues
about AutoHandle. I was first looking for threads, and only
then including googletest, so no matter the value of
gtest_disable_pthreads, it failed.

The other obvious solution is for user to first include
googletest, and only then look for threads by himself.
Fix WhenSorted() documentation example
This turns on optimization which allows the compiler to discover more
problems and omit some more warnings.
This allows doing things like TEST_P(TestFixture, MAYBE(TestName)) for nicer conditional test disabling.
When using INSTANTIATE_TEST_CASE_P with a lambda function which uses
'info' as parameter name, GCC complains that this would shadow
parameter 'info' used in the macro's VA_ARGS call.
As mentioned in issue google#360:
"Now that all the platforms gtest supports work with value-parameterized
tests, we should remove the uses of the GTEST_HAS_PARAM_TESTS macro from
the codebase everywhere."
google#360
It's not necessary, as the target_link_libraries command contains an
absolute path already, and the path given doesn't exist anymore,
leading only to linker warnings like:
ld: warning: directory not found for option
'-L/Users/travis/build/google/googletest/build/googlemock/gtest/src'
@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Jan 18, 2018
@dirk-thomas dirk-thomas self-assigned this Jan 18, 2018
Copy link

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm, +1 for force pushing master, though it will be disruptive to existing checkouts.

@dirk-thomas dirk-thomas merged commit 4b6e624 into master Jan 18, 2018
@dirk-thomas
Copy link
Author

I force-pushed the changes to the ros2 branch and synced the master branch with the same state of upstream.

@dirk-thomas dirk-thomas deleted the rebase_upstream_master branch January 18, 2018 21:05
@dirk-thomas dirk-thomas mentioned this pull request Jan 19, 2018
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in review Waiting for review (Kanban column)
Projects
None yet
Development

Successfully merging this pull request may close these issues.