SDK bump, new ad units, separate load and show for interstitials #312

redbrogdon · 2017-12-16T20:56:10Z

Hello from the AdMob DevRel team!

I'm making some updates to the firebase-admob plugin as a first step toward teaching myself Flutter. This is my first time coding in Dart and my first Flutter-related commit, so feel free to critique anything. :)

redbrogdon · 2017-12-16T21:40:17Z

TIL about proper line length for .dart files.

redbrogdon · 2017-12-17T00:42:14Z

The remaining Travis failure is for the iOS build:

Found "Xcode 8.3.3". Xcode 9.0 or greater is required to develop for iOS.
Encountered error while building for device.

I don't see this error for any of the other PRs, but I also haven't touched anything specific to iOS here. Any advice?

lukef · 2017-12-17T00:43:59Z

@redbrogdon - unrelated, you may want to bump gradle to 3.0.1 as that is the latest. i.e.: com.android.tools.build:gradle:3.0.1

redbrogdon · 2017-12-17T00:55:00Z

Nice catch. Done.

lukef · 2017-12-17T01:07:33Z

re: xcode. It might be related to cocoapods. The example does not specify a platform value but travis is set up to use xcode 8.3. If cocoapods is being downloaded then it could be defaulting to 9.x.

The example app should probably specify a platform value and the travis configuration should also probably be updated to use xcode 9.

redbrogdon · 2017-12-17T03:57:30Z

Hrm. Xcode is still failing, and I don't want to try messing with the Travis configuration for a project for which I'm not yet a contributor

If anyone from the Flutter team can point me toward the best way to resolve this, much obliged. I'll hold here and work on some other changes locally for now.

zanderso · 2017-12-18T06:27:14Z

/cc @mravn-google

mravn-google · 2017-12-18T07:22:41Z

We're currently seeing the same Xcode 8 vs Xcode 9 issue for all plugins (I believe) due to flutter/flutter@7fb7852. We seem to need an upgrade to the Travis config for flutter/plugins. I'm on it (#314)

mravn-google

Thanks for contributing!

mravn-google · 2017-12-18T08:33:00Z

packages/firebase_admob/android/build.gradle


 buildscript {
    repositories {
+        maven {


With Gradle 4.x, you can write just google().

mravn-google · 2017-12-18T08:44:33Z

packages/firebase_admob/android/build.gradle


    dependencies {
-        classpath 'com.android.tools.build:gradle:2.3.0'
+        classpath 'com.android.tools.build:gradle:3.0.1'


This is a breaking change, right? We are eventually going to move all plugins to Gradle 4.1 / Android Studio Gradle plugin 3.0.1, but by doing so, existing Flutter projects cannot consume them until they also upgrade these Gradle-related dependencies. This is OK, we just need to make sure that each plugin's version is bumped accordingly (semver), and that the change log includes an entry stating the compatibility break and how to deal with it. You may wish to link to our wiki page describing how to upgrade existing Flutter projects.

Reverted. See my comment below on the firebase-core and -ads versions.

mravn-google · 2017-12-18T08:44:53Z

packages/firebase_admob/android/build.gradle


 rootProject.allprojects {
    repositories {
+        maven {


mravn-google · 2017-12-18T08:45:43Z

packages/firebase_admob/android/build.gradle

-    compileSdkVersion 25
-    buildToolsVersion '25.0.3'
+    compileSdkVersion 26
+    buildToolsVersion '26'


The new Flutter template uses buildToolsVersion '26.0.3' to avoid an Android Studio warning about a newer version being available.

mravn-google · 2017-12-18T08:46:23Z

packages/firebase_admob/example/android/app/build.gradle

-    compileSdkVersion 25
-    buildToolsVersion '25.0.3'
+    compileSdkVersion 26
+    buildToolsVersion '26'


mravn-google · 2017-12-18T08:48:29Z

packages/firebase_admob/example/android/app/build.gradle

-    androidTestCompile 'com.android.support:support-annotations:25.2.0'
-    androidTestCompile 'com.android.support.test:runner:0.5'
-    androidTestCompile 'com.android.support.test:rules:0.5'
+    androidTestImplementation 'com.android.support:support-annotations:26.1.0'


New Flutter project template uses the following, roughly aligning with the project templates of Android Studio 3.0.1:

dependencies { testImplementation 'junit:junit:4.12' androidTestImplementation 'com.android.support.test:runner:1.0.1' androidTestImplementation 'com.android.support.test.espresso:espresso-core:3.0.1' }

mravn-google · 2017-12-18T08:49:29Z

packages/firebase_admob/example/ios/Podfile

@@ -1,5 +1,5 @@
 # Uncomment this line to define a global platform for your project
-# platform :ios, '9.0'
+platform :ios, '8.0'


This is inconsistent with new Flutter requirements, I believe (flutter/flutter@7fb7852).

This was just an attempt to try to unbreak the Travis Xcode test. It's reverted now.

mravn-google · 2017-12-18T09:28:19Z

packages/firebase_admob/android/build.gradle

-        compile 'com.google.firebase:firebase-core:11.4.+'
-        compile 'com.google.firebase:firebase-ads:11.4.+'
+        // firebase-core is exposed so the google-services plugin will see it.
+        api 'com.google.firebase:firebase-core:11.6.2'


I'm a bit uneasy about pinning these dependencies to specific versions. Many Flutter apps are going to be using several Firebase-related plugins together, and then they face the challenge of ensuring consistency among transient dependencies like firebase-core. Since the other plugins currently (and unfortunately) use 11.4.+, we have an inconsistency with 11.6.2. Is upgrading to 11.6.2 necessary? If not, please stay at 11.4.+. Otherwise, we need to provide consistent versions of the other Firebase plugins as well. Those updates could then bump the Gradle versions as well. And I'd suggest using the version range specifier [11.6.2,12.0[ to avoid a similar issue in the future, at least until somebody needs v12.

Cloud Firestore also needs a bump to 11.6.+, see comment.

Right, this plugin doesn't exist in a vacuum, and is instead part of a suite that needs to work together. I should have accounted for that.

I don't need any recently released features, so there's no need to update the versions in this file now. When they do get updated, it should clearly be in concert with the others. Change reverted.

redbrogdon · 2017-12-18T20:41:09Z

Thanks for the quick feedback! I believe I've addressed your concerns, and with the Travis config change 9.2 is building the iOS version just fine. Let me know if I missed anything or there are other issues, though.

On a related note, I've begun adding rewarded video ads to the plugin in another local branch. Would it be a normal practice to send a work-in-progress PR with just the Dart code (no iOS or Android platform code), so someone can sanity check it and the API before I keep going?

mravn-google · 2017-12-18T20:45:31Z

@redbrogdon Sounds great! If you create a PR that represents work in progress, just mark it as such in the title (e.g. [WIP] Xxx or [Review only] Yyy) and add a comment saying what you'd like reviewers to focus on and what to ignore for now.

lukef · 2017-12-19T06:29:08Z

@redbrogdon @mravn-google - It might also make sense to bump the compile + target sdk to 27 and build tools to 27.0.2 as they are the latest versions. Also, I know we're using inclusive dependency versioning, but it's worth noting that Google Play Services 11.8 came out today as well so all firebase deps will use 11.8.

mravn-google · 2017-12-19T08:24:51Z

@lukef Our current strategy is to follow Android Studio templates as much as possible/reasonable. And those are still at v26, I believe (at least, that is the case on my machine). Not sure why, but possibly related to the fact that the SDK manager says v27 is only "partially installed", offering me no obvious way to complete it.

mravn-google · 2017-12-19T10:51:25Z

@redbrogdon @lukef FYI, #317 is on its way too, with Gradle and dependency upgrades to Firebase-related plugins.

…#309)

lukef · 2017-12-19T18:25:01Z

re: templates - I think the AS templates probably haven't caught up to the release yet. It's trivial to change them for each app, but last night I got a notification that flutter run is installing SDK platform 25 because it needs 25 (outdated) meaning that my app is now dependent on many versions of the SDK and build tools. This is less than ideal as it takes up space on laptops, creates possible conflicts, etc. It would be ideal if you could target an SDK for all plugins + flutter + native that is synchronized but i understand the difficulties of that.

re: partially installed - you need to click the "Show Package Details" checkbox at the bottom of the list. This will show the full set of options.

Add multiple database support to firebase_database. Includes Android bug fixes for firebase_core.

mravn-google · 2017-12-20T06:55:06Z

@redbrogdon Sorry, we should get you in a position to land this. May I ask you to write a CHANGELOG.md entry and bump the version in pubspec.yaml? I'll deal with the Gradle version and template issues as part of #317.

redbrogdon · 2017-12-20T07:16:34Z

Sure thing. I'm doing some traveling for the holidays, but can have it for you in a day or two.

sethladd · 2017-12-20T15:44:57Z

@redbrogdon thank you so much for the contribution!

lukef · 2017-12-20T16:10:16Z

Sorry to be a pest, but does this mean that flutter will be targeting SDK 26 as a maximum? this seems contrary to what we should be doing on the android native side. I would think that targeting 27 is in the best interests of everyone, unless you know something i don't.

mravn-google · 2017-12-20T16:51:19Z

@lukef No. The project templates are not normative; they are intended to provide a starting point for new projects, and certainly the target SDK is the choice of the app developer (so long as you stay above the minimum supported by Flutter, SDK 16).

I postponed the template upgrade to version 27 because there were already too many moving pieces with the upgrade to Gradle 4.1 and and AS Gradle plugin 3.0.1.

lukef · 2017-12-20T17:03:11Z

@mravn-google - Happy to jump on a call to discuss, but .. the issue I see is that if you lock the plugin to 26 and (specifically) build-tools 26.0.2 and, lets say, you have only 26.0.1 installed then you will need to pull down v 26.0.2 on top also. so if plugin A is 26.0.1 and plugin B is 26.0.2 and your app is 27.0.2 then you have 3 versions in play.

I'm working on a loose proposal to work around it, but in the meantime my thoughts are if you push people to the highest version it is better. it'd be better for a developer to be forced to bump the build version in their app vs having to fight each plugin if they want to use 27.

lukef · 2017-12-20T17:06:14Z

Full disclosure: I don't know fully the direct consequences of the above scenario other than it using a bunch of bandwidth and disk space. There are other side effects that are best shared in an email because its probably making this PR messy :)

redbrogdon · 2017-12-22T18:04:01Z

Oh, look at me forgetting to pull changes.

redbrogdon · 2017-12-22T18:57:45Z

Clearly I'm learning as much about developing on GitHub as I am about Dart and Flutter. It looks like I've doubled up commits while rebasing onto the changes from master, which has bloated the PR and made it look (to my untrained eye) impossible to review.

Is there a simple way to fix this that you can recommend, or should I just blow away this pull request and make a new, clean one?

redbrogdon · 2017-12-28T23:51:42Z

I'm going to go ahead and close this PR and create a new one. Should be ready in a jiffy.

redbrogdon · 2017-12-29T00:25:12Z

New pull requested submitted as #327.

SDK bump, new ad units, new button

ff35833

googlebot added the cla: yes label Dec 16, 2017

Linting AdMob example's main.dart

fb308f0

Bumping Android Gradle plugin version.

9a52cf6

AdMob gradle versions now current, iOS platform specified

55fed63

Bump Xcode version to follow Flutter requirements (#314)

3bba412

mravn-google reviewed Dec 18, 2017

View reviewed changes

mravn-google and others added 2 commits December 18, 2017 15:23

Gradle upgrade for travis and battery plugin (#315)

76f3f71

Backing out breaking gradle version updates

9b750ad

Add new firebase_core plugin (#310)

8664e6e

mravn-google and others added 4 commits December 19, 2017 13:48

Basic error event communication (#311)

570932f

Enable access to the video size (#318)

8337af1

video_player version 0.0.7 (#319)

e8e5dfe

Continues removing the assumption that there is a foreground activity (…

4da5565

…#309)

Multiple database support for firebase_database (#316)

68bb455

Add multiple database support to firebase_database. Includes Android bug fixes for firebase_core.

mravn-google added 3 commits December 20, 2017 10:25

Gradle upgrade to firebase plugins (#317)

589dfae

Gradle 4.0 update (#320)

40c75d6

Gradle 4.1 update (#321)

e47f8ad

Changelog and pubspec update for firebase-admob.

05f6783

redbrogdon added 6 commits December 22, 2017 10:22

Merging.

e356da4

Linting AdMob example's main.dart

26429e1

Merging again.

319c848

Backing out breaking gradle version updates

0d9a167

Changelog and pubspec update for firebase-admob.

5eb0108

Merging.

8727c5d

redbrogdon closed this Dec 28, 2017

redbrogdon mentioned this pull request Dec 29, 2017

Adds platform-specific IDs, new "show interstitial" button to AdMob example #327

Merged

SDK bump, new ad units, separate load and show for interstitials #312

SDK bump, new ad units, separate load and show for interstitials #312

Uh oh!

Conversation

redbrogdon commented Dec 16, 2017

Uh oh!

redbrogdon commented Dec 16, 2017

Uh oh!

redbrogdon commented Dec 17, 2017

Uh oh!

lukef commented Dec 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

redbrogdon commented Dec 17, 2017

Uh oh!

lukef commented Dec 17, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

redbrogdon commented Dec 17, 2017

Uh oh!

zanderso commented Dec 18, 2017

Uh oh!

mravn-google commented Dec 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mravn-google left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mravn-google Dec 18, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

redbrogdon commented Dec 18, 2017

Uh oh!

mravn-google commented Dec 18, 2017

Uh oh!

lukef commented Dec 19, 2017

Uh oh!

mravn-google commented Dec 19, 2017

Uh oh!

mravn-google commented Dec 19, 2017

Uh oh!

lukef commented Dec 19, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mravn-google commented Dec 20, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lukef commented Dec 17, 2017 •

edited

Loading

lukef commented Dec 17, 2017 •

edited

Loading

mravn-google commented Dec 18, 2017 •

edited

Loading

mravn-google Dec 18, 2017 •

edited

Loading

lukef commented Dec 19, 2017 •

edited

Loading

mravn-google commented Dec 20, 2017 •

edited

Loading

lukef commented Dec 20, 2017 •

edited

Loading

mravn-google commented Dec 20, 2017 •

edited

Loading

lukef commented Dec 20, 2017 •

edited

Loading