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.

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] make objcOptions/cppOptions optional #4756

Closed

Conversation

andreasgangso
Copy link

Since objcOptions and cppOptions are nullable, this makes it a bit safer. Could also be moved to the constructor.

Just a hotfix for making pigeon_build_runner work without a prefix for objc, as these lines throw a null exception right now, but I made a PR to that plugin too to work around this: rotorgames/pigeon_build_runner#1

This is such a minor change and not important enough for me to bother opening an issue.
More a heads up to the pigeon maintainer.

Pre-launch Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use dart format.)
  • I signed the [CLA].
  • The title of the PR starts with the name of the package surrounded by square brackets, e.g. [shared_preferences]
  • I listed at least one issue that this PR fixes in the description above.
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes].
  • I updated CHANGELOG.md to add a description of the change, [following repository CHANGELOG style].
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • All existing and new tests are passing.

Since objcOptions and cppOptions are nullable, this makes it a bit safer. Could also be moved to the constructor.

Just a hotfix for making pigeon_build_runner work without a prefix for objc, as these lines throw a null exception right now,  but I made a PR to that plugin too to work around this: rotorgames/pigeon_build_runner#1
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat (don't just cc him here, he won't see it! He's on Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@andreasgangso andreasgangso changed the title fix: make objcOptions optional [pigeon] make objcOptions optional Aug 22, 2023
@andreasgangso andreasgangso changed the title [pigeon] make objcOptions optional [pigeon] make objcOptions/cppOptions optional Aug 22, 2023
@tarrinneal
Copy link
Contributor

This is such a minor change and not important enough for me to bother opening an issue.
More a heads up to the pigeon maintainer.

@andreasgangso If you don't feel like managing the pr (tests, versioning, etc), I can roll this into my latest pr.

auto-submit bot pushed a commit that referenced this pull request Aug 28, 2023
Adds support for enums as parameters and return types.

Dart, C++, Java, Kotlin, Objective-C, and swift:
Primitive synchronous non-null
Primitive synchronous nullable
Primitive asynchronous non-null
Primitive asynchronous nullable
Primitive Flutter api non-null
Primitive flutter api nullable

Objective-C:
Class property nullable 

Also fixes an issue with nullable enums in classes on objc. This fix required a breaking change that nested all nullable enums in a wrapper class to allow nullability. 

Also adds the ability to format files before tests run (to make my life better)

Also replaces #4756

Also adds objc prefixes to enums

fixes: flutter/flutter#87307
fixes: flutter/flutter#118733
@tarrinneal
Copy link
Contributor

Closing as changes were added with #4580

@tarrinneal tarrinneal closed this Aug 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants