[Pigeon] Condenses serialization formats #2745
Conversation
There was a problem hiding this comment.
I gave this a quick look and it's all in the right place. One thing to make sure is that there is an explicit defined order of the fields when they are written. Writing them in the order they show up in the pigeon file is good. I'd just make sure that's the case.
packages/pigeon/platform_tests/ios_swift_unit_tests/ios/RunnerTests/AsyncHandlersTest.swift
Outdated
Show resolved
Hide resolved
- Fix API call typo. - Fix double nesting of arrays for class serialization. - Replace incorrect emplace calls with push_back. - Start to update unit tests.
| @@ -588,7 +588,7 @@ const flutter::StandardMessageCodec& ${api.name}::GetCodec() { | |||
| indent.write('if ($encodableArgName.IsNull()) '); | |||
| indent.scoped('{', '}', () { | |||
| indent.writeln( | |||
| 'wrapped.emplace(flutter::EncodableValue("${Keys.error}"), WrapError("$argName unexpectedly null."));'); | |||
| 'wrapped = WrapError("$argName unexpectedly null.");'); | |||
There was a problem hiding this comment.
Note for future PR work: this is something we should remove, and replace with a debug-only assert, since per previous offline discussion we don't want non-debug runtime checks that the wire format is correct. Could you see if we have a bug about removing runtime type/nullability checks in our deserialization across any generators that have them, and file it if not?
There was a problem hiding this comment.
Issue filed, feel free to add more detail if you think my description is lacking. flutter/flutter#116972
tarrinneal
added
the
autosubmit
auto-submit
bot
removed
the
autosubmit
|
auto label is removed for flutter/packages, pr: 2745, due to - The status or check suite ios-build_all_packages CHANNEL:master has failed. Please fix the issues identified (or deflake) before re-applying this label. |
tarrinneal
added
the
autosubmit
Condenses the serialization platform of generated api's by converting objects to lists
List which issues are fixed by this PR. You must list at least one issue.
fixes flutter/flutter#111503
fixes flutter/flutter#111722
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
tbc
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.