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

[ABW-2484] Mono target #842

Merged
merged 97 commits into from
Oct 16, 2023
Merged

[ABW-2484] Mono target #842

merged 97 commits into from
Oct 16, 2023

Conversation

CyonAlexRDX
Copy link
Contributor

@CyonAlexRDX CyonAlexRDX commented Oct 15, 2023

Jira ABW-2484

Description

This PR completely removes SPM - Swift Package Manager - and moves to a pure .xcodeproj Xcode based solution, and not as a multiplatform app, but as a iOS only app. What this means is that all the source code is put in one single (app) target, instead of ~100 targets we had before.

The only behaviour changes of this PR is using the changes here #811
Otherwise the app should behave exactly as it did before.

Rationale

Why do this? Lets start with answering the question: "Why did we use many targets and SPM to begin with?" - we did it because we based our architecture on Isowords app, which felt logical since we use Pointfree's TCA, and Isowords is their showcase app for TCA.

It has now been 15 months since our first commit and we have been six iOS developers in this code base, and we have all felt the pains that super-many-targets architecture entails, which is:

  • Long build time
  • ‼️ Some times Xcode does not use latest code in a target => we think we run the bug fix code we use wrote, but in fact it was not used
    • This resulted in that I automatically ALWAYS clean build, which takes 3min 30 sec on my Mac Studio, every time, instead of a ~10 sec incremental build, since I have wasted hours on Xcode running old code...
  • Long time resolving packages - Xcode is unresponsive for a long time
  • Having many test targets instead of one results in Simulator being shutdown between tests, greatly increasing run time of tests

Result

1. clean build run time is reduced by 50%

2. clean build test time on CI is reduced by 55%

3. Logs in CI is reduced by is reduced by 55%

4. Effect of drastic reduce CI run time, is a lot of saved money for RDX works 🤑

Additional Changes

  • I also got rid of most #if os(iOS) which we have talked about a lot
  • I've updated swiftformat config to use swift 5.9 which changed from {}() -> {} in lots of places, and also removed superfluous return.
  • I've removed mac from fastlane
  • Our Preview Apps have not yet been migrated, can be done later.
  • I had to disambiguate Decimal in GatewayAPI folder to use Foundation.Decimal, manually, meaning this change will be overriden once we run generate GW models. We should fix this in a coming PR.

Copy link
Contributor

@GhenadieVP GhenadieVP left a comment

Choose a reason for hiding this comment

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

LGTM!

"revision" : "33f7e93be5d4ec027d42af77a8ec4680d1862ad2",
"version" : "11.6.4"
"revision" : "93c8dc78fbc0aa3538a0db460eb389d4180af242",
"version" : "11.3.1"
Copy link
Contributor

Choose a reason for hiding this comment

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

wanted downgrade?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • unintentional, will fix

@@ -1,12 +1,4 @@
import AppPreferencesClient
import AsyncExtensions
import ClientPrelude
import ComposableArchitecture // actually CasePaths... but CI fails if we do `import CasePaths` 🤷‍♂️
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor - would this still fail if importing just CasePaths with this PR>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

nope, I dont think so, I can try it post this PR!

Base automatically changed from ABW-2426_keychain_revamp to main October 16, 2023 10:51
@CyonAlexRDX CyonAlexRDX merged commit b800545 into main Oct 16, 2023
5 checks passed
@CyonAlexRDX CyonAlexRDX deleted the mono_target branch October 16, 2023 11:33
@CyonAlexRDX CyonAlexRDX mentioned this pull request Oct 16, 2023
@CyonAlexRDX CyonAlexRDX changed the title Mono target [ABW-2484] Mono target Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants