Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

@redbrogdon
Copy link
Contributor

This is a draft PR containing changes to the developer-facing Dart API for AdMob that will add support for rewarded video ads. It's built on top of the changes in PR #327, so those commits show up here as well. The only important code to look at is in firebase_admob.dart, though, since that contains the interface.

Some questions I'd love to get answers for:

  • Is the singleton pattern done correctly? I stole the approach from FirebaseAdMob rather than using the "factory" style.
  • Is the "id" being passed back and forth across channel calls a standard Flutter thing, or is that just a design choice by the plugins team? Because rewarded video is done with a singleton in the native Android and iOS Mobile Ads SDKs, I'm only ever going to have one object on the platform side to talk to. Is it okay to just drop that parameter when making calls (since I'll be ignoring it anyway), or would that look weird to a reader?
  • Is there anything else in here that looks like a Dart amateur wrote it -- non-idiomatic, clunky, or just plain wrong? :)

mravn-google and others added 10 commits January 2, 2018 11:36
* Update README.md

Added detail about the need to configure firebase for each platform project: Android and iOS

* Update README.md
Demo GIF image doesn't work on pub site because link is relative rather than absolute. Need to resubmit the readme for the video package.
* Add missing `break`

Without the break statement, `addDocumentListener` was falling through to `removeListener`.

* Remove todo

* Update CHANGELOG.md

* Update pubspec.yaml
* Use safe area layout to place ad in iOS 11

* Fix indentation for the format test

* Factor pre-iOS11 placement code into its own method

* Remove extra spaces for format test and improve the compiler directive

* Add larsenthomasj@gmail.com to AUTHORS

* Update version to 0.2.1

* Add 0.2.1 entry
…xample (flutter#327)

* Updates to firebase-admob example

* Correcting CHANGELOG punctuation

* Update to version 0.2.2
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this State. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@googlebot googlebot added cla: no and removed cla: yes labels Jan 5, 2018
@redbrogdon
Copy link
Contributor Author

Closing this to create a final PR.

@redbrogdon redbrogdon closed this Jan 7, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants