Skip to content

Conversation

@natebosch
Copy link
Contributor

The current version is incompatible with the latest analyzer, which is a blocker for getting the latest mockito, which is a blocker for getting the latest test_api, which is pinned by flutter_test and will be breaking in the next package roll.

The current version is incompatible with the latest `analyzer`, which is
a blocker for getting the latest `mockito`, which is a blocker for
getting the latest `test_api`, which is pinned by `flutter_test` and
will be breaking in the next package roll.
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@stuartmorgan-g
Copy link
Collaborator

stuartmorgan-g commented Mar 27, 2023

@stuartmorgan-g
Copy link
Collaborator

Never mind, I forgot this is still publish_to: none

Copy link
Member

@gmackall gmackall left a comment

Choose a reason for hiding this comment

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

LGTM

I've also actually tried running the sample app with the generated code from this version of pigeon already (because I was curious about the generated FlutterError exception in 9.1.0), and I didn't run into any issues.

@stuartmorgan-g
Copy link
Collaborator

because I was curious about the generated FlutterError exception in 9.1.0

It doesn't do anything unless you use it. You can throw it instead of a generic RuntimeException in order to better control how the Dart-side PlatformException is populated from it, since the default RuntimeException->PlatformException translation isn't all that helpful in making good errors.

@Hixie
Copy link
Contributor

Hixie commented Mar 27, 2023

test-exempt: dependency roll

@gmackall
Copy link
Member

because I was curious about the generated FlutterError exception in 9.1.0

It doesn't do anything unless you use it. You can throw it instead of a generic RuntimeException in order to better control how the Dart-side PlatformException is populated from it, since the default RuntimeException->PlatformException translation isn't all that helpful in making good errors.

Yes, I didn't realize that the Runtime->Platform was the default approach and it felt weird enough to me that I was looking around at what else was possible, but I ended up just sticking with it.

@stuartmorgan-g
Copy link
Collaborator

If you're returning any errors in camerax, I definitely recommend updating to using FlutterError everywhere when you have a chance :)

@natebosch natebosch added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 27, 2023
@auto-submit auto-submit bot merged commit 3347a31 into flutter:main Mar 27, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 28, 2023
@natebosch natebosch deleted the camerax-latest-pigeon branch March 28, 2023 17:41
nploi pushed a commit to nploi/packages that referenced this pull request Jul 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App needs tests p: camera platform-android

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants