-
Notifications
You must be signed in to change notification settings - Fork 373
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
feat(pubsub): install pubsub_mocks pkg #10008
Conversation
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Codecov ReportBase: 93.96% // Head: 93.97% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #10008 +/- ##
=======================================
Coverage 93.96% 93.97%
=======================================
Files 1513 1513
Lines 139964 139964
=======================================
+ Hits 131524 131532 +8
+ Misses 8440 8432 -8
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
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 not sure this is breaking either:
- How would a customer use the mocks library before? Did it work? If the answer is "it could not work" then this is not breaking, you are fixing a broken thing.
To test this, we need a thing in the cmake-install
build that uses it. We typically hijack the quickstarts for that purpose, but maybe you need something more ad-hoc.
Maybe you could extend ci/verify_current_targets
to also check the mocks?
@@ -0,0 +1,19 @@ | |||
# Copyright 2020 Google LLC |
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.
nit: copyright year
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.
Fixed.
# limitations under the License. | ||
|
||
include(CMakeFindDependencyMacro) | ||
find_dependency(google_cloud_cpp_pubsub) |
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.
if this was something like:
find_dependency(@GOOGLE_CLOUD_CPP_WE_NEED_A_NAME@)
then this file could be a generic file shared by all the libraries.
Maybe for a future 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.
I have taken a mental note.
We would probably use a macro for just the service name (e.g. pubsub) because we include @[email protected]
below.
|
||
include(CMakeFindDependencyMacro) | ||
find_dependency(google_cloud_cpp_pubsub) | ||
find_dependency(gtest) |
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.
There is a bit of a mess with FindGTest
(the built-in module in CMake) vs. the config file installed by googletest:
https://cmake.org/cmake/help/latest/module/FindGTest.html
Maybe we should install a file like this and include it instead?
https://github.com/googleapis/google-cloud-cpp/blob/main/cmake/FindGMockWithTargets.cmake
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.
As you pointed out, there is incremental progress to be made by installing in cases where FindGTest
works. We can figure out how to test the other case in our CI later.
0ec42be
to
dcab21d
Compare
Google Cloud Build Logs
ℹ️ NOTE: Kokoro logs are linked from "Details" below. |
Part of the work for #5782
google-cloud-cpp::pubsub_mocks
out ofpubsub-targets
and intopubsub_mocks-targets
.I have no idea if this works. Or how to test it.
But I can (from the
demo-fedora-pr
docker shell)docker:demo-fedora$ ls /h/google-cloud-cpp-installed/lib64/cmake/google_cloud_cpp_pubsub_mocks/ google_cloud_cpp_pubsub_mocks-config-version.cmake pubsub_mocks-targets.cmake google_cloud_cpp_pubsub_mocks-config.cmake # the pubsub one has a `noconfig.cmake`... whatever that means.
docker:demo-fedora$ PKG_CONFIG_PATH="/h/google-cloud-cpp-installed/lib64/pkgconfig:${PKG_CONFIG_PATH:-}" docker:demo-fedora$ pkg-config --validate google_cloud_cpp_pubsub_mocks
of course,
GTest
is not installed on that image, so I don't know what is being validated. /shrugThis change is