-
Notifications
You must be signed in to change notification settings - Fork 124
Remove LoadAdResult from new Admob. #723
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
Remove LoadAdResult from new Admob. #723
Conversation
@@ -186,6 +190,10 @@ class AdResult { | |||
/// Gets the message describing the error. | |||
const std::string& message() const; | |||
|
|||
/// Gets the ResponseInfo if an loadAd error occurred, with a collection of |
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.
Will there always be a responseinfo? What if it's the situation where it wasn't, will it just be empty?
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.
Yes, the call to this method will return an emtpy ResponseInfo object.
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.
Updated the comments.
❌ Integration test FAILEDRequested by @DellaBitta on commit efda224
Add flaky tests to go/fpl-cpp-flake-tracker |
Description
Remove LoadAdResult and fold it's data into AdResult.
The ResponseInfo member of LoadAdResult now resides in AdResult and will only be populated in Results that stem from
loadAd
requests.Note: With this new class / data member topography, there isn't an order in which I could declare AdResult in types.h without the compiler complaining that it needed a definition of its ResponseInfo member. I solved this by making the member a pointer to a ResponseInfo and this made the compiler very happy. We're friends now.
Testing
Local testing on iOS and Android phones.
iTestRun
Type of Change
Place an
x
the applicable box:Notes
N/A.