Skip to content
This repository has been archived by the owner on Jun 13, 2023. It is now read-only.

Updating build in preparation for iOS 13 update. #210

Merged
merged 47 commits into from
Nov 30, 2019
Merged

Updating build in preparation for iOS 13 update. #210

merged 47 commits into from
Nov 30, 2019

Conversation

noobs2ninjas
Copy link
Member

@noobs2ninjas noobs2ninjas commented Sep 15, 2019

This contains changes for issue #194 and is preparation for issue #209.

@noobs2ninjas
Copy link
Member Author

Finally! After SEVERAL HOURS I found out the whole reason nothing worked is because the latest version of Jazzy 0.11.0 is broken and xcodebuild_arguments doesnt work. Anyway, this is finally passing and should be everything we need to build for xcode 11/iOS 13

@drdaz
Copy link
Member

drdaz commented Nov 28, 2019

Nailed it.

Can I get push access to your repo @noobs2ninjas?

@drdaz
Copy link
Member

drdaz commented Nov 28, 2019

So it looks like I had push access. I pushed, and now CI can't check the code out 😩

@drdaz
Copy link
Member

drdaz commented Nov 28, 2019

Æh.

Something to do with ruby on the CI environment it seems... I didn't change anything in the source to alter this afaik.

@drdaz
Copy link
Member

drdaz commented Nov 28, 2019

CircleCI is ignoring the .ruby-version file on this PR for some reason. It's set to 2.6.3, but the server is trying to set it to something older and failing.

If anybody knows the right way to touch it up to make it happy, please do. Pretty sure the tests would work if they could run 😃

@drdaz
Copy link
Member

drdaz commented Nov 29, 2019

Boom! 🙂

@drdaz drdaz requested a review from TomWFox November 29, 2019 16:48
@drdaz
Copy link
Member

drdaz commented Nov 29, 2019

I think we're good to go here now.

Once @TomWFox has given this another look, can we push a release? This has been incompatible with the main SDK for a really long time.

EDIT: It seems I bumped the minor version number for no reason at all. sigh

Copy link
Contributor

@TomWFox TomWFox left a comment

Choose a reason for hiding this comment

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

Nice work, looks ok to me, love to see those green ticks 😍

Only query I have is why has the Facebook-objc-SDK been added to the Carthage config? - apologies if it’s already been mentioned but I haven’t been able to keep up with the whole thread!

@TomWFox
Copy link
Contributor

TomWFox commented Nov 29, 2019

Also, As you’ve bumped the version number it would make sense to do the changelog in this pr too.

@drdaz
Copy link
Member

drdaz commented Nov 29, 2019

Nice work, looks ok to me, love to see those green ticks 😍

Only query I have is why has the Facebook-objc-SDK been added to the Carthage config? - apologies if it’s already been mentioned but I haven’t been able to keep up with the whole thread!

It hasn't been added to the Carthage config; the Cartfile doesn't contain it. It presumably gets resolved as a dependency of the main SDK. This wasn't happening previously?

Also, As you’ve bumped the version number it would make sense to do the changelog in this pr too.

That would make sense I guess. I'll see if I can figure that out.

@TomWFox
Copy link
Contributor

TomWFox commented Nov 29, 2019

True, it shows up in the cartfile.resolved and the .gitmodules files. I don’t think it should be a dependency of the main SDK only the parseui and Facebook-Utils submodules?

@drdaz
Copy link
Member

drdaz commented Nov 29, 2019

True, it shows up in the cartfile.resolved and the .gitmodules files. I don’t think it should be a dependency of the main SDK only the parseui and Facebook-Utils submodules?

Carthage doesn't support submodules, sadly. So the way the main SDK is structured, using Carthage, you get the whole shebang, dependencies and all.

Copy link
Contributor

@TomWFox TomWFox left a comment

Choose a reason for hiding this comment

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

Got it, never used Carthage so it's workings baffle me somewhat. I've just made fix and a couple changes to the changelog to bring it in line with other repos.

@drdaz drdaz merged commit f4f5a8b into parse-community:master Nov 30, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants