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

T 1404636: Fix pipeline issue #172

Closed
wants to merge 1 commit into from

Conversation

amrutakamat-mindbody
Copy link
Contributor

This pull request includes (pick all that apply):

  • Bugfixes
  • New features
  • Breaking changes
  • Documentation updates
  • Unit tests
  • Other

Summary

Fix pipeline failing issue by removing outdated checks for Touch ID availability on older versions of macOS and iOS.

Implementation

  • Removed the check for touchIDNotAvailable for versions prior to macOS 10.13 and iOS 11.

Test Plan

  1. Test App and verify pipeline is successful

Copy link

@asolovev asolovev left a comment

Choose a reason for hiding this comment

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

Even though this warning was present in the most recent pipeline run, the run itself failed because of a failed test: ConduitTests.OAuth2RequestPipelineMiddlewareTests testNotifiesMigratorPreAndPostFetchTokenHooksForRefreshes

@@ -57,10 +57,7 @@ public final class KeychainHybridKeyProvider: HybridKeyProvider {
guard let errorCode = error.flatMap({ LAError.Code(rawValue: $0.code) }) else {
return false
}
if #available(OSX 10.13, iOS 11, *) {
Copy link

Choose a reason for hiding this comment

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

Since we declare compatibility with iOS 9 in Package.swift, I don't think we can remove this availability check.

Copy link

Choose a reason for hiding this comment

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

If you still want to fix the warning, I am not sure that this #availability check is correct. You can try something like if #available(iOS 10, macOS 10.13, *) instead

@amrutakamat-mindbody
Copy link
Contributor Author

The changes are not required

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.

2 participants