Skip to content
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: Use versions from firebase_core pubspec #351

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

frederikstonge
Copy link

Description

There are TODO above gradle plugin versions of google services, crashlytics and performance to get them from the firebase_core pubspec definition.

❌❌ We might need to wait for this to be merged to avoid breaking changes : firebase/flutterfire#16944 ❌❌

Type of Change

  • feat -- New feature (non-breaking change which adds functionality)
  • 🛠️ fix -- Bug fix (non-breaking change which fixes an issue)
  • ! -- Breaking change (fix or feature that would cause existing functionality to change)
  • 🧹 refactor -- Code refactor
  • ci -- Build configuration change
  • 📝 docs -- Documentation
  • 🗑️ chore -- Chore

@CLAassistant
Copy link

CLAassistant commented Jan 6, 2025

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions bot added the Needs Attention OP created or responded to issue and it needs attention. label Jan 6, 2025
@russellwheatley
Copy link
Member

This LGTM. CI fails because it is a fork. I ran tests locally and it was successful:

 melos run test --no-select                                                       
Building package executable... 
Built melos:melos.
melos run test
  └> melos exec --ignore="flutterfire_starter_hooks" --concurrency=1 -- "dart pub get && dart run test --reporter expanded"
     └> RUNNING

$ melos exec
  └> dart pub get && dart run test --reporter expanded
     └> RUNNING (in 1 packages)

-------------------------------------------------------------------------------------------------------------------------------------------------------------
flutterfire_cli:
Resolving dependencies...
Downloading packages...
  dart_console 1.2.0 (4.1.1 available)
  intl 0.18.1 (0.20.1 available)
  pub_updater 0.4.0 (0.5.0 available)
  pubspec 2.3.0 (discontinued replaced by pubspec_parse)
Got dependencies!
1 package is discontinued.
3 packages have newer versions incompatible with dependency constraints.
Try `dart pub outdated` for more information.
00:00 +0: loading test/install_test.dart
00:00 +0: test/install_test.dart: Success on basic install
00:05 +1: test/reconfigure_test.dart: flutterfire configure: android - "default" Apple - "default"
00:08 +2: test/reconfigure_test.dart: flutterfire configure: android - "default" Apple - "default"
00:21 +3: test/reconfigure_test.dart: flutterfire configure: android - "default" Apple - "default"
00:22 +4: test/reconfigure_test.dart: flutterfire configure: android - "default" Apple - "default"
00:28 +5: test/reconfigure_test.dart: flutterfire configure: android - "default" Apple - "default"
00:32 +6: test/configure_test.dart: flutterfire configure: android - "build configuration" Apple - "build configuration"
00:41 +7: test/reconfigure_test.dart: flutterfire configure: android - "build configuration" Apple - "build configuration"
01:00 +8: test/configure_test.dart: flutterfire configure: android - "default" Apple - "target"
01:00 +9: test/reconfigure_test.dart: flutterfire configure: android - "default" Apple - "target"
01:21 +10: test/reconfigure_test.dart: flutterfire configure: android - "default" Apple - "target"
01:27 +11: test/configure_test.dart: flutterfire configure: rewrite service files when rerunning "flutterfire configure" with different apps
01:54 +12: test/configure_test.dart: flutterfire configure: test when only two platforms are selected, not including "web" platform
02:05 +13: test/configure_test.dart: flutterfire configure: test will reconfigure project if no args and `firebase.json` is present
02:22 +14: test/configure_test.dart: flutterfire configure: write Dart configuration file to different output
02:40 +15: test/configure_test.dart: flutterfire configure: incorrect `--web-app-id` should throw exception
02:47 +16: test/configure_test.dart: flutterfire configure: incorrect `--windows-app-id` should throw exception
02:53 +17: test/configure_test.dart: flutterfire configure: get correct Firebase App with manually created Firebase web app via `--web-app-id`
03:01 +18: test/configure_test.dart: flutterfire configure: get correct Firebase App with manually created Firebase web app via `--windows-app-id`
03:08 +19: test/configure_test.dart: flutterfire configure: check Dart file configuration is updated correctly
03:45 +20: test/configure_test.dart: flutterfire configure: ensure android build.gradle files are only updated once
03:57 +21: test/configure_test.dart: flutterfire configure: path with spaces should not break the configuration for iOS/macOS
04:08 +22: All tests passed!
flutterfire_cli: SUCCESS

The most important test is this one: https://github.com/invertase/flutterfire_cli/blob/main/packages/flutterfire_cli/test/configure_test.dart#L1417-L1421

Which checks versions are ported over correctly.

@russellwheatley
Copy link
Member

Wait until FlutterFire's next release before we merge this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocked: do-not-merge Needs Attention OP created or responded to issue and it needs attention.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants