[pigeon] Don't wrap non-nullable primitives in Obj-C #5214
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tarrinneal
approved these changes
Oct 30, 2023
Contributor
tarrinneal
left a comment
tarrinneal
left a comment
There was a problem hiding this comment.
Looks good, I like the changes you made to the typing class
| /// Returns a version of the string form that can be used directly before | ||
| /// another string (e.g., a variable name) and handle spacing correctly for | ||
| /// a right-aligned pointer format. | ||
| String get beforeString => hasAsterisk ? toString() : '$this '; |
Contributor
There was a problem hiding this comment.
is toString() just providing an empty string here? we have a ton of '' in the generators.
Collaborator
Author
There was a problem hiding this comment.
It's calling the toString method that's just above this, giving '$basename *'; it just looks odd because usually toString() isn't being called on this.
stuartmorgan-g
added
the
autosubmit
Collaborator
Author
|
Once this is published I'll update all of our plugins, so the updates aren't done as an incidental part of other changes when we are much less likely to notice implicit conversion bugs. |
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Oct 30, 2023
engine-flutter-autoroll
added a commit
to engine-flutter-autoroll/flutter
that referenced
this pull request
Oct 31, 2023
auto-submit bot
pushed a commit
to flutter/flutter
that referenced
this pull request
Oct 31, 2023
flutter/packages@2af6954...c9fec61 2023-10-31 [email protected] [image_picker] Prevent multiple active calls on iOS (flutter/packages#5272) 2023-10-30 [email protected] [in_app_purchase_android] Add missing response code to BillingResponse enum (flutter/packages#5120) 2023-10-30 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.22.4 to 2.22.5 (flutter/packages#5263) 2023-10-30 [email protected] Bump play-service-maps dependency from version 18.1.0 to 18.2.0 (flutter/packages#5243) 2023-10-30 [email protected] Roll Flutter from a4ec627 to e12d1a7 (3 revisions) (flutter/packages#5267) 2023-10-30 [email protected] [camerax] Loosen restrictions of fallback strategies for choosing resolutions (flutter/packages#5239) 2023-10-30 [email protected] [pigeon] Don't wrap non-nullable primitives in Obj-C (flutter/packages#5214) 2023-10-30 [email protected] [flutter_lints] Replace "flutter pub add --dev" with "dev:" (flutter/packages#5260) 2023-10-28 [email protected] Roll Flutter from 5907c97 to a4ec627 (18 revisions) (flutter/packages#5255) 2023-10-27 [email protected] [go_router] Fixes deep-link with no path on cold start (flutter/packages#5113) 2023-10-27 [email protected] [two_dimensional_scrollables] Fix pinned row painting when one axis is reversed (flutter/packages#5187) 2023-10-27 [email protected] Roll Flutter from c555599 to 5907c97 (45 revisions) (flutter/packages#5252) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
ksokolovskyi
pushed a commit
to ksokolovskyi/flutter
that referenced
this pull request
Oct 31, 2023
flutter/packages@2af6954...c9fec61 2023-10-31 [email protected] [image_picker] Prevent multiple active calls on iOS (flutter/packages#5272) 2023-10-30 [email protected] [in_app_purchase_android] Add missing response code to BillingResponse enum (flutter/packages#5120) 2023-10-30 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 2.22.4 to 2.22.5 (flutter/packages#5263) 2023-10-30 [email protected] Bump play-service-maps dependency from version 18.1.0 to 18.2.0 (flutter/packages#5243) 2023-10-30 [email protected] Roll Flutter from a4ec627 to e12d1a7 (3 revisions) (flutter/packages#5267) 2023-10-30 [email protected] [camerax] Loosen restrictions of fallback strategies for choosing resolutions (flutter/packages#5239) 2023-10-30 [email protected] [pigeon] Don't wrap non-nullable primitives in Obj-C (flutter/packages#5214) 2023-10-30 [email protected] [flutter_lints] Replace "flutter pub add --dev" with "dev:" (flutter/packages#5260) 2023-10-28 [email protected] Roll Flutter from 5907c97 to a4ec627 (18 revisions) (flutter/packages#5255) 2023-10-27 [email protected] [go_router] Fixes deep-link with no path on cold start (flutter/packages#5113) 2023-10-27 [email protected] [two_dimensional_scrollables] Fix pinned row painting when one axis is reversed (flutter/packages#5187) 2023-10-27 [email protected] Roll Flutter from c555599 to 5907c97 (45 revisions) (flutter/packages#5252) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Closed
12 tasks
16 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Updates the Objective-C generator to use
BOOL,NSInteger, anddoublefor non-nullable versions of those primitives, instead of boxing them withNSNumber*. This makes the code more idiomatic, brings them into alignment with enum behavior, and importantly helps minimize the chances of clients having issues with silent conversion ofNSNumber*toBOOLin the long term (although it may cause some such issues in the short term when people update; the changelog contains a prominent warning about this).As an incidental fix, this also fixes the use of
strongfor collections and strings, which is not best practice, since I was already changing related code.Fixes flutter/flutter#116680
Fixes flutter/flutter#127401
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.///).