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

[local_auth_android] update java complie sdk version to green tree #7121

Merged
merged 2 commits into from
Feb 7, 2023
Merged

[local_auth_android] update java complie sdk version to green tree #7121

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ed458d5
update java complie sdk version
tarrinneal Feb 7, 2023
e4052b9
again
tarrinneal Feb 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/local_auth/local_auth/example/android/app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ apply plugin: 'com.android.application'
apply from: "$flutterRoot/packages/flutter_tools/gradle/flutter.gradle"

android {
compileSdkVersion 31
compileSdkVersion 33

lintOptions {
disable 'InvalidPackage'
Expand Down
2 changes: 1 addition & 1 deletion packages/local_auth/local_auth_android/example/android/app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ apply plugin: 'com.android.application'
apply from: "$flutterRoot/packages/flutter_tools/gradle/flutter.gradle"

android {
compileSdkVersion 31
compileSdkVersion 33
Copy link
Contributor

Choose a reason for hiding this comment

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

The red is on local_auth compilation, not local_auth_android. That's why it was post-publish (from c9f6bee); if it were in the _android subpackage it would have shown up in presubmit. So we'll need to update local_auth's example.

@camsim99 we'll need to be careful about updating the examples of app-facing packages as well when we do this kind of dependabot roll.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, shoot I see. Sorry about missing that. So, in the future, we need to bump in the app facing package as well as the android sub-package?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The red is on local_auth compilation, not local_auth_android. That's why it was post-publish (from c9f6bee); if it were in the _android subpackage it would have shown up in presubmit. So we'll need to update local_auth's example.

This passes the tests now, should I update both?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah, that test isn't running here.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, shoot I see. Sorry about missing that.

I didn't think of it either. Unfortunately we still have holes in presubmit testing of things that only break dependencies, because running all of our tests twice would be a lot. But it seems like we still need to find the right line.

So, in the future, we need to bump in the app facing package as well as the android sub-package?

If we know something will affect any clients of the plugin, then all clients in our repo (which is usually just the app-facing package; occasionally we have cross-plugin dependencies in examples, like camera's example depending on video_player) should be updated.

The trick is knowing when it will affect clients, but having to update the in-package example is a good warning sign.

Copy link
Contributor

Choose a reason for hiding this comment

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

That makes sense. Would it be helpful to provide some guidance on this in the wiki? I'm not sure how many folks will run into this, though.

Copy link
Contributor

Choose a reason for hiding this comment

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

It wouldn't hurt, but I suspect we'll hit it infrequently enough that we'll forget even if we write it down.

I left flutter/flutter#52115 (comment) to capture considering adding CI that would catch this.


lintOptions {
disable 'InvalidPackage'
Expand Down