Skip to content

[pigeon] Allows swift generator to skip error class generation - #7849

Merged
auto-submit[bot] merged 11 commits into
flutter:mainfrom
wamynobe:swift-error-sharing
Oct 23, 2024
Merged

[pigeon] Allows swift generator to skip error class generation#7849
auto-submit[bot] merged 11 commits into
flutter:mainfrom
wamynobe:swift-error-sharing

Conversation

@wamynobe

@wamynobewamynobe commented Oct 11, 2024

Copy link
Copy Markdown
Contributor

Since the Kotlin generator allows skipping error class generation, it makes sense for the Swift generator to have the same option.

Related to:
#6183
flutter/flutter#142099

Pre-launch Checklist

If you need help, consider asking for advice on the #hackers-new channel on Discord.

@google-cla

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@wamynobe

Copy link
Copy Markdown
ContributorAuthor

Hi @tarrinneal,
In your previous PR #6183 , there are a few parts that I don't really understand (mostly about how to generate test files), for example the code kotlinIncludeErrorClass: input != 'core_tests', in packages/pigeon/tool/shared/generation.dart. Could you please guide me through these test cases?

@wamynobe
wamynobe marked this pull request as ready for review October 17, 2024 07:06
@wamynobe

Copy link
Copy Markdown
ContributorAuthor

looks like the test cases using the default error class name in Swift have failed.

@wamynobe

Copy link
Copy Markdown
ContributorAuthor

How can I run those custom_package_tests test cases on my local machine?

@tarrinneal

Copy link
Copy Markdown
Contributor

if you navigate to the pigeon directory you can run dart run tool/test.dart -f -t ios_swift_integration_tests

@tarrinneal

tarrinneal commented Oct 17, 2024

Copy link
Copy Markdown
Contributor

This code

 final bool kotlinErrorClassGenerationTestFiles =
input == 'core_tests' || input == 'background_platform_channels';
final String kotlinErrorName = kotlinErrorClassGenerationTestFiles
? 'FlutterError'
: '${pascalCaseName}Error';

and the corresponding usage of kotlinErrorName ensures that there are two files with the same error class name. You'll need to do that for swift as well.

@wamynobe

wamynobe commented Oct 18, 2024

Copy link
Copy Markdown
ContributorAuthor

if you navigate to the pigeon directory you can run dart run tool/test.dart -f -t ios_swift_integration_tests

Thanks @tarrinneal I am now able to run those test cases.

and the corresponding usage of kotlinErrorName ensures that there are two files with the same error class name. You'll need to do that for swift as well.

I saw swiftErrorUseDefaultErrorName which has the same purpose as kotlinErrorClassGenerationTestFiles. Updated!

@tarrinnealtarrinneal left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Thanks for putting this together! (looks like I forgot to submit my approval last week, sorry)

@tarrinnealtarrinneal added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Oct 23, 2024
@tarrinneal

Copy link
Copy Markdown
Contributor

@stuartmorgan for 2nd review

@stuartmorgan-gstuartmorgan-g left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@stuartmorgan-gstuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 23, 2024
@auto-submit
auto-submitBot merged commit ff82995 into flutter:mainOct 23, 2024
auto-submitBot pushed a commit to flutter/flutter that referenced this pull request Oct 24, 2024
flutter/packages@5e03bb1...a556f0f
2024-10-24 10687576+bparrishMines@users.noreply.github.com [camera] Manual roll and skip failing tests (flutter/packages#7891)
2024-10-23 49699333+dependabot[bot]@users.noreply.github.com [image_picker]: Bump androidx.annotation:annotation from 1.8.2 to 1.9.0 in /packages/image_picker/image_picker_android/android (flutter/packages#7898)
2024-10-23 49699333+dependabot[bot]@users.noreply.github.com [url_launcher]: Bump androidx.annotation:annotation from 1.8.2 to 1.9.0 in /packages/url_launcher/url_launcher_android/android (flutter/packages#7906)
2024-10-23 48524817+wamynobe@users.noreply.github.com [pigeon] Allows swift generator to skip error class generation (flutter/packages#7849)
2024-10-23 49699333+dependabot[bot]@users.noreply.github.com [in_app_pur]: Bump androidx.annotation:annotation from 1.8.2 to 1.9.0 in /packages/in_app_purchase/in_app_purchase_android/android (flutter/packages#7903)
2024-10-23 49699333+dependabot[bot]@users.noreply.github.com [file_selector]: Bump androidx.annotation:annotation from 1.8.2 to 1.9.0 in /packages/file_selector/file_selector_android/android (flutter/packages#7900)
2024-10-23 43054281+camsim99@users.noreply.github.com [video_player_android] Give `can pause` integration test range to test for (flutter/packages#7913)
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 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
creatorpiyush pushed a commit to creatorpiyush/packages that referenced this pull request Jun 10, 2026
…er#7849)
Since the Kotlin generator allows skipping error class generation, it makes sense for the Swift generator to have the same option.
Related to:
flutter#6183flutter/flutter#142099
bisor0627 pushed a commit to bisor0627/packages that referenced this pull request Jun 19, 2026
…er#7849)
Since the Kotlin generator allows skipping error class generation, it makes sense for the Swift generator to have the same option.
Related to:
flutter#6183flutter/flutter#142099
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmitMerge PR when tree becomes green via auto submit Appp: pigeon

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@wamynobe@tarrinneal@stuartmorgan-g