Skip to content

Revert ios build workaround for Firebase packages #1388

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

Merged
merged 1 commit into from
Mar 20, 2025

Conversation

rajveermalviya
Copy link
Member

This reverts commit a9b2016.

@rajveermalviya rajveermalviya requested a review from PIG208 March 7, 2025 18:08
@rajveermalviya rajveermalviya added the maintainer review PR ready for review by Zulip maintainers label Mar 7, 2025
Copy link
Member

@PIG208 PIG208 left a comment

Choose a reason for hiding this comment

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

Thanks! Looks like I missed this in my review list. I wasn't able to test this manually; from reading the changes and the linked context, this LGTM. Marking this for Greg's review.

@PIG208 PIG208 assigned gnprice and unassigned PIG208 Mar 19, 2025
@PIG208 PIG208 requested a review from gnprice March 19, 2025 00:50
@PIG208 PIG208 added integration review Added by maintainers when PR may be ready for integration and removed maintainer review PR ready for review by Zulip maintainers labels Mar 19, 2025
Copy link
Member

@gnprice gnprice left a comment

Choose a reason for hiding this comment

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

Thanks for cleaning this up!

Let's keep around the Zulip.xcconfig file and the bits of config that wire it up — it'll just become empty except for the general comment at top.

That way the next time there's some Xcode config we want to add, we have all the infrastructure in place to just drop it in that file and have it get applied. Effectively we'll get to re-use the work that we did in #958 to figure those details out.

@rajveermalviya rajveermalviya force-pushed the pr-remove-ios-build-workaround branch from 7e64e82 to 623b8f1 Compare March 20, 2025 11:22
@rajveermalviya
Copy link
Member Author

Thanks for the review @gnprice! Pushed a new revision, PTAL.

@rajveermalviya rajveermalviya requested a review from gnprice March 20, 2025 11:24
This reverts part of commit a9b2016 (zulip#958),
keeping the `Zulip.xcconfig` file and its references.

The fix was released in version `3.6.0` of `package:firebase_core`
and version `15.1.3` of `package:firebase_messaging`:
  firebase/flutterfire@d7d2d4b

We have since then updated Firebase dependencies 3 times:
  zulip@24215f6
  zulip@1abb0a0
  zulip@3ba9985
@gnprice
Copy link
Member

gnprice commented Mar 20, 2025

Thanks! Looks good; merging, with a few commit-message tweaks:

-    Revert "ios: Add workaround for package:firebase_messaging when building with Xcode 16"
+    ios build: Revert workaround for package:firebase_messaging
 
-    This reverts part of commit a9b20167ccf13ebaf6bcd6a28060ca4438f8c8a8,
-    keeping the `Zulip.xcconfig` file and it's references.
+    This reverts part of commit a9b20167c (#958),
+    keeping the `Zulip.xcconfig` file and its references.

(In particular, 'Revert "ios: …"' is no longer accurate because it's not reverting that whole commit.)

@gnprice gnprice force-pushed the pr-remove-ios-build-workaround branch from 623b8f1 to e3b274a Compare March 20, 2025 21:58
@gnprice gnprice merged commit e3b274a into zulip:main Mar 20, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
integration review Added by maintainers when PR may be ready for integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants