Skip to content
This repository has been archived by the owner on Nov 10, 2023. It is now read-only.

[apple_asset_catalog] Include Product Type in AcTool Args #2403

Merged
merged 2 commits into from
Apr 14, 2020

Conversation

ydxia
Copy link
Contributor

@ydxia ydxia commented Feb 24, 2020

Currently, building Messages extensions with buck will not generate the Messages app icon correctly. This is because in order to do so, actool needs to know what product type it is creating an asset catalog + app icon for.

This change funnels the product type from apple_bundle into AppleAssetCatalog and AcToolStep so that actool is called with --product-type <product type> (needs to be com.apple.product-type.app-extension.messages for Messages extensions).

@facebook-github-bot
Copy link
Contributor

Hi @ydxia!

Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@ydxia
Copy link
Contributor Author

ydxia commented Feb 28, 2020

@v-jizhang can you take a look? 🙏

@v-jizhang
Copy link
Contributor

@ydxia We are working on a change in buck open source, please see the announcement #2401. And we will get back to you soon. Thanks you.

@ydxia
Copy link
Contributor Author

ydxia commented Mar 3, 2020

For my understanding, should changes target the dev branch then? The announcement references changes made by the "Facebook’s Build Infra team" for dev; does this impact how external contributions are merged?

@v-jizhang
Copy link
Contributor

Yes, all PRs will be reviewed and merged here in the master branch. Only selected PRs can be ported to dev.

@ydxia
Copy link
Contributor Author

ydxia commented Mar 3, 2020

ok so dev will contain experimental changes to infrastructure whereas bug fixes should still go to master?

@v-jizhang
Copy link
Contributor

Bug fixes should be ported to dev too.

@ydxia
Copy link
Contributor Author

ydxia commented Mar 10, 2020

hi - just pinging again here to get a sense of what the expected workflow is and if there's anything needed from my side. Thanks!

@v-jizhang
Copy link
Contributor

Your PR looks good to me but we 're working on an Open Source plan. Thank you!

@v-jizhang v-jizhang requested review from zpao and removed request for asp2insp March 24, 2020 23:07
@v-jizhang v-jizhang self-requested a review April 14, 2020 21:55
@v-jizhang v-jizhang merged commit dcfb5af into facebook:master Apr 14, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants