Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[pigeon] isEnum, isClass, fix swift casting, default values, optional method arguments, named method arguments #5355

Merged
merged 19 commits into from
Nov 17, 2023
Merged

[pigeon] isEnum, isClass, fix swift casting, default values, optional method arguments, named method arguments #5355

merged 19 commits into from
Nov 17, 2023

Conversation

tarrinneal
Copy link
Contributor

@tarrinneal tarrinneal commented Nov 8, 2023 •
edited
Loading

  • Breaking change [dart] Renames locally defined host API variables.
    • Host api static field codec changed to pigeonChannelCodec.
  • Adds named parameters to host API methods.
  • Adds optional parameters to host API methods.
  • Adds default values for class constructors and host API methods.
  • Adds isEnum and isClass to TypeDeclarations.
  • Removes all klass variable names.

fixes flutter/flutter#127719
fixes flutter/flutter#98448
fixes flutter/flutter#80048

stuartmorgan
stuartmorgan requested changes Nov 13, 2023
View reviewed changes
packages/pigeon/CHANGELOG.md Outdated Show resolved Hide resolved
packages/pigeon/CHANGELOG.md Outdated Show resolved Hide resolved
packages/pigeon/example/app/lib/src/messages.g.dart Show resolved Hide resolved
packages/pigeon/pigeons/core_tests.dart Outdated Show resolved Hide resolved
packages/pigeon/lib/ast.dart Outdated Show resolved Hide resolved
packages/pigeon/lib/ast.dart Outdated Show resolved Hide resolved
packages/pigeon/lib/ast.dart Outdated Show resolved Hide resolved
packages/pigeon/lib/dart_generator.dart Outdated Show resolved Hide resolved
packages/pigeon/lib/dart_generator.dart Show resolved Hide resolved
packages/pigeon/lib/pigeon_lib.dart Outdated Show resolved Hide resolved
stuartmorgan
stuartmorgan approved these changes Nov 17, 2023
View reviewed changes
Copy link
Contributor

@stuartmorgan stuartmorgan left a comment

Choose a reason for hiding this comment

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

LGTM with some small comments.

(For future reference, this would have been better broken into two or three PRs IMO. In general, mixing lots of boilerplate changes (hundreds of lines of them in this case) with actual functional changes is a good sign that something should be separated if feasible.)

packages/pigeon/lib/ast.dart Outdated Show resolved Hide resolved
packages/pigeon/lib/dart_generator.dart Show resolved Hide resolved
packages/pigeon/lib/dart_generator.dart Outdated Show resolved Hide resolved
packages/pigeon/lib/dart_generator.dart Outdated Show resolved Hide resolved
packages/pigeon/lib/dart_generator.dart Outdated Show resolved Hide resolved
packages/pigeon/lib/dart_generator.dart Outdated Show resolved Hide resolved
packages/pigeon/lib/generator_tools.dart Outdated Show resolved Hide resolved
packages/pigeon/lib/generator_tools.dart Outdated Show resolved Hide resolved
@tarrinneal tarrinneal mentioned this pull request Nov 17, 2023
Closed
11 tasks
@tarrinneal tarrinneal added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 17, 2023
@auto-submit auto-submit bot merged commit db86e8f into flutter:main Nov 17, 2023
80 checks passed
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Nov 20, 2023
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 20, 2023
@engine-flutter-autoroll
db86e8f [pigeon] isEnum, isClass, fix swift casting, default values, …
a310a78 
…optional method arguments, named method arguments (flutter/packages#5355)
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 20, 2023
@engine-flutter-autoroll
Roll Packages from 07b4b29 to c5443ad (13 revisions) (#138724)
16fc837 
flutter/packages@07b4b29...c5443ad

2023-11-20 JeroenWeener@users.noreply.github.com [webview_flutter] Support for handling basic authentication requests (Platform Interface) (flutter/packages#5362)
2023-11-18 engine-flutter-autoroll@skia.org Roll Flutter from 53a57ad to 6cf9ab0 (31 revisions) (flutter/packages#5426)
2023-11-18 kevmoo@users.noreply.github.com [shared_preferences_web] migrate to pkg:web (flutter/packages#5414)
2023-11-18 stuartmorgan@google.com [ci] Roll minimum allowable Flutter to 3.10 (flutter/packages#5425)
2023-11-18 43054281+camsim99@users.noreply.github.com [path_provider_android]  Run tests on AVDs running Android 34 (flutter/packages#5222)
2023-11-17 tarrinneal@gmail.com [pigeon] isEnum, isClass, fix swift casting, default values, optional method arguments, named method arguments (flutter/packages#5355)
2023-11-17 stuartmorgan@google.com [plugin_platform_interface] Switch mixin to `mixin class` (flutter/packages#5420)
2023-11-17 stuartmorgan@google.com [go_router] Fixes use of `Iterable` (flutter/packages#5421)
2023-11-17 stuartmorgan@google.com [pigeon] Adds `analyzer` 6.x compatibility (flutter/packages#5418)
2023-11-17 stuartmorgan@google.com Update release step to 3.16 (flutter/packages#5416)
2023-11-17 kevmoo@users.noreply.github.com [file_selector_web] migrate to pkg:web (flutter/packages#5413)
2023-11-16 stuartmorgan@google.com [video_player] Improve macOS frame management (flutter/packages#5078)
2023-11-16 engine-flutter-autoroll@skia.org Roll Flutter from e8c2bb1 to 53a57ad (39 revisions) (flutter/packages#5412)

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 flutter-ecosystem@google.com,rmistry@google.com 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
HugoOlthof pushed a commit to moneybird/packages that referenced this pull request Dec 13, 2023
@tarrinneal @HugoOlthof
[pigeon] isEnum, isClass, fix swift casting, default values, optional…
976bf4e 
… method arguments, named method arguments (flutter#5355)

* **Breaking change** [dart] Renames locally defined host API variables.
  * Host api static field `codec` changed to `pigeonChannelCodec`.
* Adds named parameters to host API methods.
* Adds optional parameters to host API methods.
* Adds default values for class constructors and host API methods.
* Adds `isEnum` and `isClass` to `TypeDeclaration`s.
* Removes all `klass` variable names.

fixes flutter/flutter#127719
fixes flutter/flutter#98448
fixes flutter/flutter#80048
@reidbaker reidbaker mentioned this pull request Jan 12, 2024
Closed
@reidbaker reidbaker mentioned this pull request Jan 19, 2024
Merged
12 tasks
@reidbaker reidbaker mentioned this pull request Feb 19, 2024
Merged
16 tasks
arc-yong pushed a commit to Arctuition/packages-arc that referenced this pull request Jun 14, 2024
@tarrinneal @arc-yong
[pigeon] isEnum, isClass, fix swift casting, default values, optional…
3db38ee 
… method arguments, named method arguments (flutter#5355)

* **Breaking change** [dart] Renames locally defined host API variables.
  * Host api static field `codec` changed to `pigeonChannelCodec`.
* Adds named parameters to host API methods.
* Adds optional parameters to host API methods.
* Adds default values for class constructors and host API methods.
* Adds `isEnum` and `isClass` to `TypeDeclaration`s.
* Removes all `klass` variable names.

fixes flutter/flutter#127719
fixes flutter/flutter#98448
fixes flutter/flutter#80048
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@stuartmorgan stuartmorgan stuartmorgan approved these changes

Assignees
No one assigned
Labels
autosubmit Merge PR when tree becomes green via auto submit App p: pigeon platform-android platform-ios platform-macos platform-windows
Projects
None yet
Milestone
No milestone
2 participants
@tarrinneal @stuartmorgan