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

Have cocoapods rebuild FacebookSDKStrings.bundle so xcode processes strings files #1713

Closed

Conversation

cltnschlosser
Copy link
Contributor

@cltnschlosser cltnschlosser commented Apr 10, 2021

The end result is the final product ends up converting the strings files from the current text format to a minified binary plist format.

Size change:
Before
246,293 bytes (365 KB on disk)

After
133,039 bytes (184 KB on disk)

Nice ~100KB win for installed app size.

I decided to leave the bundle alone instead of breaking out the strings into their own folder because I didn't know if anything else was using the bundle currently. It doesn't look like spm is using it, which is probably a bug.

Also I'm not sure if these strings have test coverage or not, but if not I can help make sure various translations are still working correctly. Not sure where exactly these strings are used.

To help us review the request, please complete the following:

  • sign contributor license agreement
  • I've ensured that all existing tests pass and added tests (when/where necessary) Wasn't sure how to run them all, I assume PR process has CI that will run them.
  • I've updated the documentation (when/where necessary) and Changelog (when/where necessary)
  • I've added the proper label to this pull request (e.g. bug for bug fixes) I can't add labels.

@facebook facebook deleted a comment from 123gid Apr 11, 2021
…trings files

The end result is the final product ends up converting the strings files from the current text format to a minified binary plist format.

Size change:
Before
246,293 bytes (365 KB on disk)

After
133,039 bytes (184 KB on disk)

Nice ~100KB win for installed app size.

I decided to leave the bundle alone instead of breaking out the strings into their own folder because I didn't know if anything else was using the bundle currently. It doesn't look like spm is using it, which is probably a bug.

Also I'm not sure if these strings have test coverage or not, but if not I can help make sure various translations are still working correctly. Not sure where exactly these strings are used.
@cltnschlosser cltnschlosser force-pushed the cs_binaryPlistStrings branch from 6b1e171 to a115663 Compare April 16, 2021 21:10
@cltnschlosser cltnschlosser force-pushed the cs_binaryPlistStrings branch from a115663 to 6a30ea8 Compare April 16, 2021 21:10
@cltnschlosser
Copy link
Contributor Author

Can someone take a look at this. Thanks

@facebook-github-bot
Copy link
Contributor

@joesus has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@joesus merged this pull request in 8138f8c.

@joesus
Copy link
Contributor

joesus commented Apr 20, 2021

Thanks for the contribution! 😍

@cltnschlosser cltnschlosser deleted the cs_binaryPlistStrings branch April 20, 2021 19:28
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.

4 participants