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

Feature/Rewrite to async #24

Merged
merged 7 commits into from
May 15, 2024
Merged

Feature/Rewrite to async #24

merged 7 commits into from
May 15, 2024

Conversation

yllfejziu
Copy link
Member

@yllfejziu yllfejziu commented May 2, 2024

Note: This is a major update for PovioKitAuth. The old implementation with promises is overridden with async/await.
To be tested more for any edge cases before merging / releasing.

The following social connections need to be tested:

  • SignIn with Apple
  • SignIn with Google
  • SignIn with Facebook
  • SignIn with LinkedIn

@yllfejziu yllfejziu requested a review from a team May 2, 2024 13:52
@yllfejziu yllfejziu force-pushed the feature/rewrite-to-async branch from 56e0b0f to 3e608d0 Compare May 2, 2024 14:22
@yllfejziu yllfejziu force-pushed the feature/rewrite-to-async branch from 3e608d0 to fe280ce Compare May 10, 2024 09:33
Copy link
Collaborator

@borut-t borut-t left a comment

Choose a reason for hiding this comment

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

Awesome 🙌

Copy link
Collaborator

@borut-t borut-t left a comment

Choose a reason for hiding this comment

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

👌

/// Checks the current auth state and returns the boolean value as promise.
public var checkAuthentication: Promise<Authenticated> {
/// Checks the current auth state and returns the boolean value asynchronously.
public func checkAuthentication() async -> Authenticated {

Choose a reason for hiding this comment

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

Perhaps this function should be throwable and the error handled at the call site.

Choose a reason for hiding this comment

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

And you can leave it as a getter as well. :) Though a function is fine by me as well.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's remove this function completely. We have a mechanism in terms of notification about the revocation.

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

@yllfejziu yllfejziu requested a review from tonikocjan May 15, 2024 11:51
Copy link
Collaborator

@borut-t borut-t left a comment

Choose a reason for hiding this comment

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

👌

@yllfejziu yllfejziu merged commit 0b5e7b3 into develop May 15, 2024
1 check passed
@yllfejziu yllfejziu deleted the feature/rewrite-to-async branch May 15, 2024 12:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants