Skip to content

Conversation

toto
Copy link
Contributor

@toto toto commented Nov 5, 2019

Changes

  • Remove deprecated API for NSURLCache on catalyst
  • Remove maccatalyst. prefix from bundle ID before registering PFInstallation

The deprecated API allows the parse SDK to be linked for Mac Catalyst apps and the Xcode 11 SDKs (iOS 13, macOS 10.15, etc.) in general.

In addition to this it also removes the maccatalyst. prefix added by Catalyst from the bundle ID. The reasoning is that a Catalyst app should be treated as a iOS App just running on the Mac in all regards. If this is not done some things will not work (e.g. sending push notifications fails since the topic is wrong (using maccatalyst.com.example.app instead of com.example.app which is correct. If the SDK user wants to distinguish Catalyst installations channels are a better choice.

@toto toto changed the title Don't use api not available in catalyst Don't use NSURLCache API not available in Catalyst Nov 5, 2019
@TomWFox
Copy link
Contributor

TomWFox commented Nov 13, 2019

Hi @toto, some of the test seem to be passing now.

Is this ready for review as far as you’re concerned?

@toto
Copy link
Contributor Author

toto commented Nov 13, 2019

Functionally it should be done (I have a catalyst app using this code). I will look into the tests some more but it can be reviewed.

@TomWFox
Copy link
Contributor

TomWFox commented Nov 13, 2019

Cool, I haven’t used used Catalyst so not in the best position to review.

@parse-community/ios-osx anyone willing & able to review this?

@mman
Copy link
Contributor

mman commented Nov 13, 2019

I am in no position to review officially but the changes look good to me and match my original pull request #1460. I have not had a chance to test the app id prefix removal but looks good as well and is definitely required to make push notifications work.

@toto
Copy link
Contributor Author

toto commented Nov 14, 2019

I fixed the macOS tests. It seems Xcode 11 defaults to parallel tests, which some of the filesystem based tests don't really like. Also the address sanatizer causes crashes during tests so I disabled it. It's a bit strange since it does not seem to be a problem in real world usage from what I can tell.

@toto
Copy link
Contributor Author

toto commented Nov 14, 2019

At some (not to distant point) I guess we need to move to the non-legacy build system. Which from a few tests does not seem a smooth transition for the test suite.

@noobs2ninjas
Copy link
Member

For some reason the travis integration failed when travis ran jazzy. Which is weird because the jazzy script shouldnt run on that test.

11.84s$ git submodule update --init --recursive
install
293.64s$ bundle install
$ ./Scripts/jazzy.sh

This is where it times out.

@toto
Copy link
Contributor Author

toto commented Nov 22, 2019

Strange. I was wondering what jazzy was doing.

In other news. I think we need to add a bit of docs regarding catalyst. The change here fixes the bundle identifier in the default configuration. However it is possible
To customize this and we should add a section to the docs I guess.

@noobs2ninjas
Copy link
Member

@toto

I think that is a great Idea. I'd be interested in hearing about Carthage because right now I'm having issues with and getting builds and dependencies to match up because of what Xcode 11 environment converting targets to use Catalyst automatically. In fact we are working right now on our own fork of Bolts because its main build doesn't work with catalyst and we cant simply use the Bolts/Task submodule. So we are having to figure out how to make this work with Carthage when building with Circle and Travis.

In fact, if you wouldn't mind could you take a look at the issue here? Although this is for the ParseLiveQuery iOS sdk but this same issue is keeping us from doing releases on the iOS Parse repo as well. Any advice would be helpful. This currently is keeping us from doing releases.

I'm going to go ahead and approve this and we can merge it in. None of the reasons builds failed had anything to do with your code.

@toto
Copy link
Contributor Author

toto commented Nov 26, 2019

@noobs2ninjas Sure I will take a look at parse-community/ParseLiveQuery-iOS-OSX#210

@noobs2ninjas
Copy link
Member

Hey @toto

I didnt realize another pull request was out for this same issue right before you did your pull request. If you want to catch the branch up and see if theres anything you can do to improve feel free.

@toto toto force-pushed the feature/maccatalyst branch from 4b916b7 to 200fb1d Compare December 6, 2019 11:27
@toto
Copy link
Contributor Author

toto commented Dec 6, 2019

@noobs2ninjas I bought up the branch to master. The fix now focusses on the bundleid prefix handling for Catalyst apps. This will correctly handle the "default" case if someone just "ticks the box" in Xcode. If the setup is custom this will not work, but I guess that is ok since in that case the change will also not break any behaviour.

@noobs2ninjas
Copy link
Member

Thanks @toto!

@noobs2ninjas noobs2ninjas merged commit d0119bd into parse-community:master Dec 10, 2019
@chih98
Copy link

chih98 commented Dec 29, 2019

Sorry if this is a redundant question, but I promise I looked.

When will the release that this feature is in be released? Where do I find the project's release schedule for future reference? Thanks!

@TomWFox
Copy link
Contributor

TomWFox commented Dec 31, 2019

We don’t have a release schedule. It would be good to get a release out soon - @drdaz @noobs2ninjas - I think we’re ready for a release? I believe the release build is still broken so we may require a manual publish to cocoa pods.

@drdaz
Copy link
Member

drdaz commented Dec 31, 2019

Hello! I'll try and get the cocoapods build fixed during the week, then there should be nothing standing in the way of a release.

I'm also pretty close to having an implementation of Sign in with Apple working in ParseUI, so maybe we can sneak that in 🙂

@TomWFox
Copy link
Contributor

TomWFox commented Dec 31, 2019

Sounds great, let me know if there is anything I can do.

Also @chih98 just thought I’d mention with cocoa pods you can always pull master or a specific commit instead of a published release.

Sent with GitHawk

@TomWFox TomWFox mentioned this pull request Apr 13, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants