-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[pigeon] Fixed decoding code for null-safe classes. #292
Conversation
97ab024
to
8a8132c
Compare
8a8132c
to
ccaf34b
Compare
ccaf34b
to
6a36235
Compare
return 'List<Object$nullTag>$nullTag'; | ||
case 'Map': | ||
return 'Map<Object$nullTag, Object$nullTag>'; | ||
return 'Map<Object$nullTag, Object$nullTag>$nullTag'; | ||
default: | ||
return dataType; | ||
return '$dataType$nullTag'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the actual fix, other stuff is just testing infrastructure / tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
makes sense
// Autogenerated from Pigeon (v0.1.21), do not edit directly. | ||
// See also: https://pub.dev/packages/pigeon | ||
// ignore_for_file: public_member_api_docs, non_constant_identifier_names, avoid_as, unused_import | ||
// @dart = 2.12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to add this file otherwise CI analysis steps fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe, we disable the analysis for flutter_null_safe_unit_tests
? https://dart.dev/guides/language/analysis-options#the-analysis-options-file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pointer. I'm on the fence as to whether it is a good idea or not. I think I'll keep it this way for now.
Let me add @blasten to this review, he pointed yesterday to the code you're touching here! |
issue: flutter/flutter#76405
Pre-launch Checklist
[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy.CHANGELOG.md
to add a description of the change.///
).If you need help, consider asking for advice on the
#hackers-new
channel on Discord.