-
Notifications
You must be signed in to change notification settings - Fork 0
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
disable https enforcement in debug mode #2
base: spm
Are you sure you want to change the base?
Conversation
This allows us to connect to the dev environment without https. Related to sensational/popscan-ios#1358
I'm not sure I'm the most appropriate reviewer for this, but from other languages I would say: what's wrong with a preprocessor check (macro?) instead of a runtime check? Calls to private APIs might be detected unless the compiler optimizes it away, with a preprocessor macro we'd be sure it gets removed |
This is what I wanted to do first and then I realised that this dependencies are built by carthage so this means we would need to rebuild it when debugging. I thought this was simpler to just add a runtime check. |
Ah I would have thought that carthage would make two separate builds, one when the main project is in Debug and one when it is in Release… Could you check that this isn't the case? It's weird because it would mean that when we run our project in Debug, we don't get a Debug build with symbols of our dependencies |
When running locally, carthage is not called at each build. We just call it once as explained in https://github.com/sensational/popscan-ios/#installation On the CI, we always start from a fresh installation as all our caching attempts failed. |
@balland do we still need this PR? Are we still using this fork of OAuth? |
I think we still need some fixes I did. @ajandrade could you confirm? |
This PR we are not using so I would say to ignore. The fork we still need to keep. |
If we keep the branch then this is still useful for me to be able to run the app with my dev environment without doing this tweak by hand 😄 |
Ah that's what this is for. I had totally forgotten in the meantime :D Seems pretty useful, what about making it official? Anyone wanting to develop the app against a local backend will need this. |
All my previous attempts to make a PR on the original repo were not a success but I can give it a try 😄 |
If you want to merge it on our side, we are using our own SPM branch: https://github.com/sensational/OAuth2/tree/spm |
As SPM should be the default now, maybe we should merge the SPM branch into master after creating a legacy branch for our older release branches that still needs carthage (in case we need to fix bugs). Anyway I guess the carthage config refers to a specific commit so the legacy branch is only need if we need to do bug fixes (very unlikely) without having the SPM changes. |
For now I have changed the base branch to spm. @ajandrade we should maybe think of make this spm branch the new master branch. Do we still need to keep the support for carthage? If not, no need to create a legacy branch. |
@ajandrade we should do the same for the |
Found in https://alexplescan.com/posts/2016/05/03/swift-a-nicer-way-to-tell-if-your-app-is-running-in-debug-mode/
As @ajandrade said, let's hope this hack is not going to disappear in
the next version of Swift. Anyway we will need to update our
dependencies and see if all our changes still apply :)