Skip to content
This repository was archived by the owner on Feb 22, 2023. It is now read-only.

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Oct 1, 2019

Description

  • Limit the supported podspec platform to iOS so tests don't run (and fail) for macOS.
  • Define the module by setting DEFINES_MODULE in the podspec. See CocoaPod modular headers docs.
  • Explicitly set VALID_ARCHS to architectures included in the Flutter universal binary. This is the CocoaPods-suggested workaround to prevent tests from running on the i386 simulator. See Fix linting when armv7 is included but i386 isn't CocoaPods/CocoaPods#8159.
  • Fix a few analyzer warnings:
    • nullability warning in FLTImagePickerMetaDataUtil
    • NSNumber warnings to check against nil or -boolValue

Related Issues

Fixes flutter/flutter#41657.
See also flutter/flutter#41007.

Tests

  • Add a test_spec to the podspec so the CocoaPods linter will test them. These tests fail before the s.platform and s.pod_target_xcconfig changes.
  • Move existing tests to new Tests directory so they are run on pre-submit!
  • Unfortunately I couldn't get the test images to embed as resources in the test_spec so load the data out of the bundle. Loads from the file from the example test app, but drop back to base64 string representation so the tests can be run without those files.

Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • My PR includes unit or integration tests for all changed/updated/fixed behaviors (See [Contributor Guide]).
  • All existing and new tests are passing.
  • I updated/added relevant documentation (doc comments with ///).
  • The analyzer (flutter analyze) does not report any problems on my PR.
  • I read and followed the [Flutter Style Guide].
  • The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
  • I updated pubspec.yaml with an appropriate new version according to the [pub versioning philosophy].
  • I updated CHANGELOG.md to add a description of the change.
  • I signed the [CLA].
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

  • Yes, this is a breaking change (please indicate a breaking change in CHANGELOG.md and increment major revision).
  • No, this is not a breaking change.

@jmagman jmagman requested a review from cyanglaz as a code owner October 1, 2019 01:40
Copy link
Contributor

@cyanglaz cyanglaz left a comment

Choose a reason for hiding this comment

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

LGTM!

@jmagman jmagman merged commit c458aa8 into flutter:master Oct 1, 2019
@jmagman jmagman deleted the module-ip branch October 1, 2019 17:28
mormih pushed a commit to mormih/plugins that referenced this pull request Nov 17, 2019
sungmin-park pushed a commit to sungmin-park/flutter-plugins that referenced this pull request Dec 17, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[image_picker] Define clang modules plugin for iOS

3 participants