-
Notifications
You must be signed in to change notification settings - Fork 24.5k
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
Hotfix privacy manifest aggregation script #44390
Conversation
@philIip has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
I picked the changes manually in 0.74.1, no need for a pick request there. We will need the pick request for 71 and 72, though. |
This pull request was successfully merged by @aleqsio in 00b3661. When will my fix make it into a release? | How to file a pick request? |
Thanks for the patch @aleqsio!
I realised why we were getting different behaviour here. There are two types of groups in Xcode; those with folders on the f/s and those without them. Groups with folders have a path, and groups without folders have a name: ![]() |
That makes sense - now we're checking for both so it should be fine, with a fallback to save to root if we find none. Thank you for reporting this so early 💛 |
Summary: As pointed out by liamjones here: facebook#44214 (comment) The original PR did introduce a bug in the `find/first` check, but in my testing, we do need to look at `group.name`, so let's make sure we check both. This also makes it play nice with an existing file even if it is added to a different directory, by appending to it instead of forcing it to exist in the main group. ## Changelog: [IOS] [FIXED] - Fix privacy aggregation Pull Request resolved: facebook#44390 Test Plan: Tested on rn-tester Reviewed By: cipolleschi Differential Revision: D56893594 Pulled By: philIip fbshipit-source-id: b92589bc2bed9d07e9af20c56a8b9f6c61d864f0
Summary: As pointed out by liamjones here: facebook#44214 (comment) The original PR did introduce a bug in the `find/first` check, but in my testing, we do need to look at `group.name`, so let's make sure we check both. This also makes it play nice with an existing file even if it is added to a different directory, by appending to it instead of forcing it to exist in the main group. ## Changelog: [IOS] [FIXED] - Fix privacy aggregation Pull Request resolved: facebook#44390 Test Plan: Tested on rn-tester Reviewed By: cipolleschi Differential Revision: D56893594 Pulled By: philIip fbshipit-source-id: b92589bc2bed9d07e9af20c56a8b9f6c61d864f0
Summary: As pointed out by liamjones here: #44214 (comment) The original PR did introduce a bug in the `find/first` check, but in my testing, we do need to look at `group.name`, so let's make sure we check both. This also makes it play nice with an existing file even if it is added to a different directory, by appending to it instead of forcing it to exist in the main group. ## Changelog: [IOS] [FIXED] - Fix privacy aggregation Pull Request resolved: #44390 Test Plan: Tested on rn-tester Reviewed By: cipolleschi Differential Revision: D56893594 Pulled By: philIip fbshipit-source-id: b92589bc2bed9d07e9af20c56a8b9f6c61d864f0
Summary:
As pointed out by @liamjones here:
#44214 (comment)
The original PR did introduce a bug in the
find/first
check, but in my testing, we do need to look atgroup.name
, so let's make sure we check both.This also makes it play nice with an existing file even if it is added to a different directory, by appending to it instead of forcing it to exist in the main group.
Changelog:
[IOS] [FIXED] - Fix privacy aggregation
Test Plan:
Tested on rn-tester