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

Update example #34

Merged
merged 16 commits into from
Aug 30, 2023
Merged

Update example #34

merged 16 commits into from
Aug 30, 2023

Conversation

nkmrh
Copy link
Collaborator

@nkmrh nkmrh commented Aug 17, 2023

Summary

Implementation

Test plan

  • Xcode と Android Studio で example アプリのビルドが成功することを確認しました

Other Information

  • CI の flutter バージョンを最新にしました 45b8031

  • melos run analyze が通るように変更しました a22a2fb

    • [karte_core]: info • 'setMockMethodCallHandler' is deprecated and shouldn't be used. Use tester.binding.defaultBinaryMessenger.setMockMethodCallHandler or TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger.setMockMethodCallHandler instead. Pass the channel as the first argument. This feature was deprecated after v3.9.0-19.0.pre • test/karte_core_test.dart:14:13 • deprecated_member_use

@nkmrh nkmrh self-assigned this Aug 17, 2023
@nkmrh nkmrh marked this pull request as draft August 17, 2023 05:48
@nkmrh nkmrh marked this pull request as ready for review August 18, 2023 01:31
@SojiroNishimura
Copy link
Contributor

@nkmrh
exampleのpubspec.yamlが更新されてないのでこのブランチに切り替えてpub getすると以下のように古いバージョンがインストールされてビルドエラーになると思うんですが新規でCloneしなおして確認していただいてもよいでしょうか?

   karte_core:
     dependency: "direct main"
     description:
-      path: "../karte_core"
-      relative: true
-    source: path
-    version: "1.2.1"
+      name: karte_core
+      sha256: c886d0e6ec5c3445d3f05c99a3b5b4ad2fe3e51fe4d2b39bb71d44dc15f9d66c
+      url: "https://pub.dev"
+    source: hosted
+    version: "0.2.0"
The method 'getUserSyncScript' isn't defined for the type 'UserSync'.
Try correcting the name to the name of an existing method, or defining a method named 'getUserSyncScript'.

repositories {
google()
mavenCentral()
}

dependencies {
classpath 'com.android.tools.build:gradle:4.1.0'
classpath 'com.android.tools.build:gradle:7.3.0'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AGP7.3.0にするとSDK側のKotlinバージョンを上げないといけなくなると思うので7.2.0に変更お願いします。SDK側に変更が入るとリリースしないといけなくなるのでSDK側を変更しなくても済む範囲でexampleの修正をお願いします。

@SojiroNishimura
Copy link
Contributor

基本的に以下の手順を試していただいてエラーが出たら修正する感じでお願いします:pray:

その他、必要に応じてexample内で完結するように修正お願いします。

@nkmrh
Copy link
Collaborator Author

nkmrh commented Aug 27, 2023

exampleのpubspec.yamlが更新されてないのでこのブランチに切り替えてpub getすると以下のように古いバージョンがインストールされてビルドエラーになると思うんですが新規でCloneしなおして確認していただいてもよいでしょうか?

@SojiroNishimura lock ファイルの diff から、おそらくビルド前に bash scripts/setup.sh を実行するとビルドできると思います。melos を使用している為、ビルド前にローカルパッケージをリンクする必要があります 🙏

example/pubspec.yamlでfirebase_messagingのバージョンをできるだけ上げる

firebase_messaging のバージョンを ^13.1.0上げたところ CI で下記エラーが発生したためバージョンの指定はそのままにしています。

Resolving dependencies in ./example...
Because karte_notification >=0.2.0 <1.1.0 depends on firebase_messaging ^10.0.1 and karte_flutter depends on firebase_messaging ^13.1.0, karte_notification >=0.2.0 <1.1.0 is forbidden.
So, because karte_flutter depends on karte_notification ^0.2.0, version solving failed.

@nkmrh nkmrh marked this pull request as draft August 27, 2023 09:35
@nkmrh nkmrh marked this pull request as ready for review August 28, 2023 09:54
@SojiroNishimura
Copy link
Contributor

SojiroNishimura commented Aug 29, 2023

exampleのpubspec.yamlが更新されてないのでこのブランチに切り替えてpub getすると以下のように古いバージョンがインストールされてビルドエラーになると思うんですが新規でCloneしなおして確認していただいてもよいでしょうか?

@SojiroNishimura lock ファイルの diff から、おそらくビルド前に bash scripts/setup.sh を実行するとビルドできると思います。melos を使用している為、ビルド前にローカルパッケージをリンクする必要があります 🙏

@nkmrh これってつまりmelosでpackagesを指定するとそのパッケージ自体が持っているpubspec.yamlは無視されてローカルパッケージの参照になるって理解であってますかね?ドキュメントと実挙動を見ると明らかに無視されてるっぽいですが分かりにくい仕様な気がしますね。。

いずれにせよパッケージ単体でpubspec.yamlとpubspec.lockの内容に齟齬があると混乱の元なのでpubspec.yamlの方も最新版を参照するように更新しておいてもらえないでしょうか。melosの仕様を知らないとハマってしまうので「melosによってを使うとローカルパッケージが参照される」旨もコメントで書いておいていただけるとありがたいです 🙏

@nkmrh
Copy link
Collaborator Author

nkmrh commented Aug 29, 2023

@SojiroNishimura

これってつまりmelosでpackagesを指定するとそのパッケージ自体が持っているpubspec.yamlは無視されてローカルパッケージの参照になるって理解であってますかね?ドキュメントと実挙動を見ると明らかに無視されてるっぽいですが分かりにくい仕様な気がしますね。。

あってます。確かにわかりにくいですね。。。

いずれにせよパッケージ単体でpubspec.yamlとpubspec.lockの内容に齟齬があると混乱の元なのでpubspec.yamlの方も最新版を参照するように更新しておいてもらえないでしょうか。melosの仕様を知らないとハマってしまうので「melosによってを使うとローカルパッケージが参照される」旨もコメントで書いておいていただけるとありがたいです 🙏

  • pubspec.yaml を更新しました 2cb98be
  • コメントを追加しました a3d9f1a

@@ -5,49 +5,56 @@ packages:
dependency: transitive
description:
name: async
url: "https://pub.dartlang.org"
sha256: "947bfcf187f74dbc5e146c9eb9c0f10c9f8b30743e341481c1e2ed3ecc18c20c"
Copy link
Collaborator Author

@nkmrh nkmrh Aug 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SojiroNishimura melos bootstrap を実行した影響で package/example 配下の pubspec.lock が更新されてしまったのですが、これらの変更は含めない方が良いでしょうか?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

対応ありがとうございます!これはexample配下なので問題ないです 👍

@SojiroNishimura
Copy link
Contributor

Dartのバージョンだけ^4.0.0に上げましたが他はLGTMなのでマージします。

@SojiroNishimura SojiroNishimura merged commit 02e1e20 into master Aug 30, 2023
1 check passed
@delete-merged-branch delete-merged-branch bot deleted the update-example branch August 30, 2023 04:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants