-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
π§ββοΈ[Xcode] Zen out build warnings #720
Conversation
@@ -5544,7 +5548,6 @@ | |||
buildSettings = { | |||
ALWAYS_EMBED_SWIFT_STANDARD_LIBRARIES = NO; | |||
ALWAYS_SEARCH_USER_PATHS = NO; | |||
ARCHS = "$(ARCHS_STANDARD)"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like that after removing this it still resolves into Standard Architecture
anyways Β―_(γ)_/Β―
@@ -177,7 +177,7 @@ internal final class ActivitiesViewController: UITableViewController { | |||
} | |||
|
|||
self.viewModel.outputs.updateUserInEnvironment | |||
.observeValues { [weak self] user in | |||
.observeValues { user in |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And it all started because of this π
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, thanks for catching this π
Can I just suggest that we delete the values in the test targets rather than set them to empty? Visually it looks like this:
Values as empty | Values removed |
---|---|
![]() |
![]() |
Then, can we also delete the 'Any iOS SDK' rows here, I think it's left over from when we once had a tvOS target:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like just this one sneaky code-signing setting is at app target level and not project level.
There are a couple more in the whole project config that are at the app target level but some of them make sense to be there. I think let's change this one seeing as it's part of a group that we've changed to all be at project level and then let's do a separate audit sometime of all the settings to ensure they're configured and inherited at the right level, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
π² What
Cleans up all outstanding Xcode warnings
π€ Why
Warnings should be an indication that something is wrong. We haven't really pay much attention to this because our project has been producing at least couple warnings in the past and therefore it's easy to introduce new ones (because we simply don't take them seriously).
π How
Updated Xcode project with Xcode's update tool
π See
π Architectures - Before
π¦ Architectures - After
π Code signing - Before
π¦ - Code signing - After
β Acceptance criteria