-
Notifications
You must be signed in to change notification settings - Fork 4.9k
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
test: add explicit ctor/dtor to more mocks #4423
Conversation
Signed-off-by: Snow Pettersen <[email protected]>
Signed-off-by: Snow Pettersen <[email protected]>
Looks like CircleCI is having trouble again. |
Yeah, I'll keep an eye on their status update and rerun CI once they recover |
Signed-off-by: Snow Pettersen <[email protected]>
@zuercher Reran the tests, we're green now! |
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.
Looks like merge conflict to resolve.
test/mocks/upstream/mocks.cc
Outdated
@@ -122,7 +130,13 @@ MockClusterUpdateCallbacks::MockClusterUpdateCallbacks() {} | |||
|
|||
MockClusterUpdateCallbacks::~MockClusterUpdateCallbacks() {} | |||
|
|||
|
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.
Seems like removing line 133 makes the formatter happy.
Signed-off-by: Snow Pettersen <[email protected]>
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.
Thanks!
Description: As per https://github.com/google/googletest/blob/master/googlemock/docs/CookBook.md#making-the-compilation-faster, moving ctor/dtor definition out of the header files can lead to faster builds.
Risk Level: Low, minor refactor
Testing: Run CI
Docs Changes: n/a
Release Notes: n/a