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

Remove use_modular_headers! from Podfiles #7796

Merged
merged 2 commits into from
Nov 5, 2024

Conversation

loic-sharma
Copy link
Member

@loic-sharma loic-sharma commented Oct 5, 2024

⚠️ This PR does not update packages' versions as it only affects unpublished parts of example apps and tests.

We added use_modular_headers! to our Podfiles as we originally planned to phase out use_frameworks! (see flutter/flutter#42204). However, our plans have now changed and we are instead phasing out CocoaPods entirely in favor of Swift Package Manager.

CocoaPods's use_frameworks! and use_modular_headers! are two different overlapping options that should not be used together. This change removes the use_modular_headers! from example apps' Podfiles. This change does not affect packages themselves.

Part of flutter/flutter#156259

Pre-launch Checklist

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

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

LGTM once the test passes

  Expected: equals [
              ProcessCall:"which pod" in /packages,
              ProcessCall:"pod lib lint /packages/plugin1/ios/plugin1.podspec --configuration=Debug --skip-tests --use-modular-headers --use-libraries" in /packages,
              ProcessCall:"pod lib lint /packages/plugin1/ios/plugin1.podspec --configuration=Debug --skip-tests --use-modular-headers" in /packages
            ] ordered
    Actual: [
              ProcessCall:"which pod" in /packages,
              ProcessCall:"pod lib lint /packages/plugin1/ios/plugin1.podspec --configuration=Debug --skip-tests --use-libraries" in /packages,
              ProcessCall:"pod lib lint /packages/plugin1/ios/plugin1.podspec --configuration=Debug --skip-tests" in /packages
            ]
     Which: at location [1] is ProcessCall:<"pod lib lint /packages/plugin1/ios/plugin1.podspec --configuration=Debug --skip-tests --use-libraries" in /packages> instead of ProcessCall:<"pod lib lint /packages/plugin1/ios/plugin1.podspec --configuration=Debug --skip-tests --use-modular-headers --use-libraries" in /packages>
  
  package:matcher                             expect
  test/podspec_check_command_test.dart 144:7  main.<fn>.<fn>

@stuartmorgan
Copy link
Contributor

⚠️ This PR only changes unpublished parts of example apps and tests. I have added the override: no versioning needed label.

Could you link to the failing run that led to adding this label? CI should not have flagged this as needing a version change.

@loic-sharma
Copy link
Member Author

loic-sharma commented Oct 7, 2024

@stuartmorgan Ah, I misunderstood the content at the bottom of this section to mean "Add the override label once you've verified your change meets these conditions": https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#version

I'll update this README to clarify this label should only be added if the CI check fails: flutter/flutter#156342

Copy link
Contributor

@chunhtai chunhtai left a comment

Choose a reason for hiding this comment

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

LGTM

@loic-sharma loic-sharma removed the override: no versioning needed Override the check requiring version bumps for most changes label Nov 4, 2024
@loic-sharma loic-sharma marked this pull request as ready for review November 4, 2024 19:41
Copy link
Contributor

@LouiseHsu LouiseHsu left a comment

Choose a reason for hiding this comment

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

in_app_purchase, local_auth, and pointer_interceptor lgtm!

@loic-sharma
Copy link
Member Author

This is now ready for review! The webview test failure that affected this change was also fixed by flutter/flutter#157449

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

@loic-sharma loic-sharma added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 5, 2024
@auto-submit auto-submit bot merged commit d3d563d into flutter:main Nov 5, 2024
76 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Nov 6, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Nov 6, 2024
flutter/packages@7219431...bb5a258

2024-11-06 [email protected] [ci] Upload screenshots, logs, and Xcode test results for drive and integration_test runs (flutter/packages#7430)
2024-11-05 [email protected] Remove use_modular_headers! from Podfiles (flutter/packages#7796)
2024-11-05 [email protected] [camera_avfoundation] enable more than 30 fps (flutter/packages#7394)
2024-11-05 [email protected] Roll Flutter from 8591d0c to 29d40f7 (25 revisions) (flutter/packages#8027)
2024-11-05 [email protected] [ci] Add vector_graphics and flutter_svg to autolabeler (flutter/packages#8025)
2024-11-05 [email protected] [vector_graphics_compiler] wasm compatibility (flutter/packages#8021)

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] 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
sinyu1012 added a commit to sinyu1012/packages that referenced this pull request Nov 8, 2024
* main: (1187 commits)
  [various] Update example app minSdkVersions (flutter#8035)
  [go_router] Activate leak testing (flutter#7546)
  [in_app_purchase_storekit] Add restore purchases and receipts (flutter#7964)
  [interactive_media_ads] Adds remaining methods for internal wrapper of the Android native `BaseManager` (flutter#7943)
  [google_sign_in/google_identity_services] Clear-up documentation of callbacks in various APIs and uses of those APIs (flutter#8029)
  [flutter_svg] wasm compatibility (flutter#8014)
  Applied Gradle Plugins Declaratively for Multiple Plugin Example Apps (Part 2) (flutter#8019)
  Roll Flutter from 29d40f7f6826 to 73546b3b71a7 (20 revisions) (flutter#8028)
  [ci] Upload screenshots, logs, and Xcode test results for drive and integration_test runs (flutter#7430)
  Remove use_modular_headers! from Podfiles (flutter#7796)
  [camera_avfoundation] enable more than 30 fps (flutter#7394)
  Roll Flutter from 8591d0c16a6c to 29d40f7f6826 (25 revisions) (flutter#8027)
  [ci] Add vector_graphics and flutter_svg to autolabeler (flutter#8025)
  [vector_graphics_compiler] wasm compatibility (flutter#8021)
  [vector_graphics*] Relax dependency constraints of vector_graphics, vector_graphics_codec, vector_graphics_compiler, flutter_svg  (flutter#8018)
  [various] Add `missing_code_block_language_in_doc_comment` lint to flutter/packages. (flutter#6473)
  [various] Update example apps to Kotlin 1.9.0 (flutter#7998)
  [go_router] add current state getter (flutter#7651)
  Applied Gradle Plugins Declaratively for Multiple Plugin Example Apps (flutter#7968)
  Roll Flutter from f86b77721524 to 8591d0c16a6c (16 revisions) (flutter#8015)
  ...

# Conflicts:
#	packages/quick_actions/quick_actions/CHANGELOG.md
#	packages/quick_actions/quick_actions_ios/CHANGELOG.md
#	packages/quick_actions/quick_actions_platform_interface/CHANGELOG.md
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.

6 participants