Skip to content

Apple user details #1551

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

Merged
merged 12 commits into from
Oct 12, 2020
Merged

Apple user details #1551

merged 12 commits into from
Oct 12, 2020

Conversation

drdaz
Copy link
Member

@drdaz drdaz commented Sep 21, 2020

This PR adds a callback to PFLoginViewController to allow developers to receive the one-time credentials and user details object from Sign In With Apple. By adding an extra callback method for this, we avoid breaking existing installs. But documentation needs updating.

@drdaz
Copy link
Member Author

drdaz commented Sep 21, 2020

Just a thought... we could also include the user object in the new callback to make it easier to assign the values.

@drdaz
Copy link
Member Author

drdaz commented Sep 21, 2020

Carthage target is broken here it seems, and Carthage is more generally broken with Xcode 12, which is what I've got installed locally. Downloading 11.7 to see if I can reproduce it.

@drdaz
Copy link
Member Author

drdaz commented Sep 25, 2020

Æh... this Travis CI - Branch target seems to be.. hesitant.

Not sure if this is because I needed to rerun some targets to get them green.

Same thing seems to be happening in #1543.

@drdaz
Copy link
Member Author

drdaz commented Sep 25, 2020

Yeah definitely meeting some resistance from CI atm

@TomWFox TomWFox closed this Sep 26, 2020
@TomWFox TomWFox reopened this Sep 26, 2020
@drdaz
Copy link
Member Author

drdaz commented Oct 5, 2020

@TomWFox About this 'Travis CI - Branch' target that never seems to complete... this is a new thing that seemed to appear along with the 'Required' labels. What should it be executing?

@drdaz
Copy link
Member Author

drdaz commented Oct 5, 2020

@cbaker6 What was the deal with codecov suddenly thinking we're down to 6%?

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 5, 2020

I'm guessing the iOS or macOS builds failed to send it's report. If so, you may have to run it again.

@@ -72,9 +72,7 @@ workflows:
jobs:
- ios
- macos
- carthage:
requires:
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe removing this may allow Carthage to run before iOS (I think the jobs start randomly and the only thing that prevents it is "requires"). Also, Carthage takes the longest, so if the iOS build fails, no reason to run Carthage because it will likely fail. Better to find out quickly that iOS failed than wait until after Carthage

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah I meant to explain myself here. My reasoning was that the iOS tests still fail randomly; that the tests fail doesn't mean the binary isn't good. And if there's been an issue with the Carthage build, working on getting it green is a crapshoot.

It's more of a hack to deal with what we've got, than the right solution.

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 5, 2020

Update, in Circle for iOS, you got a "fail to send" for codecov on the previous build

@drdaz
Copy link
Member Author

drdaz commented Oct 5, 2020

Update, in Circle for iOS, you got a "fail to send" for codecov on the previous build

Aha!

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 5, 2020

About this 'Travis CI - Branch' target that never seems to complete... this is a new thing that seemed to appear along with the 'Required' labels. What should it be executing?

This is because the branch you created for this PR exists on the parse-community repo, so it running the test there also. It should report at some point, it's probably just bring slow because these builds are slow.

@drdaz
Copy link
Member Author

drdaz commented Oct 5, 2020

About this 'Travis CI - Branch' target that never seems to complete... this is a new thing that seemed to appear along with the 'Required' labels. What should it be executing?

This is because the branch you created for this PR exists on the parse-community repo, so it running the test there also. It should report at some point, it's probably just bring slow because these builds are slow.

It shouldn't take weeks though, right?

@cbaker6
Copy link
Contributor

cbaker6 commented Oct 5, 2020

It shouldn't take weeks though, right?

Definitely not, @TomWFox can probably remove the requirement. It's probably a bug on github and travis side

@TomWFox
Copy link
Contributor

TomWFox commented Oct 6, 2020

I'll remove the requirement. Not certain but I'm pretty sure I've seen this issue of Travis CI - Branch reporting intermittently.

@drdaz drdaz requested a review from a team October 7, 2020 13:39
@drdaz drdaz removed the In Progress label Oct 7, 2020
cbaker6
cbaker6 previously approved these changes Oct 9, 2020
@mtrezza mtrezza mentioned this pull request Oct 9, 2020
@drdaz
Copy link
Member Author

drdaz commented Oct 11, 2020

Just spotted the @import. Seem to recall it causing issues in some other context in this project.

EDIT: Sry import. Didn't mean to ping you 😬.

@drdaz drdaz merged commit 0c5e999 into master Oct 12, 2020
@drdaz drdaz deleted the apple-user-details branch October 12, 2020 14:50
drdaz added a commit that referenced this pull request Oct 14, 2020
* Updates version numbers

* Updates changelog

* Apple user details (#1551)

* Adds callback to PFLoginViewController for received Apple credentials object

* Adds user to Apple credential callback

* Fixes target membership

* Cleans up target membership

* Removes CCI Carthage build dependency on iOS.
iOS tests are still temperamental.

* Removes @import

* Allow Parse SDK to be built for maccatalyst (#1543)

* allow Parse SDK to be built for maccatalyst

* Updates version numbers

* Updates changelog

* changed minimumosversion to 8.0 (#1521)

* changed minimumosversion to 8.0

closes #1515

* Updates version numbers

* Updates changelog

* Update changelog

* nits

Co-authored-by: Martin Man <[email protected]>
Co-authored-by: Manuel <[email protected]>
Co-authored-by: Tom Fox <[email protected]>
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.

3 participants