-
Notifications
You must be signed in to change notification settings - Fork 224
[Valet 4.0] Convert APIs to throw rather than return booleans or enums #198
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
Changes from all commits
Commits
Show all changes
77 commits
Select commit
Hold shift + click to select a range
fbe5b8d
Use throws rather than return types to indicate error. No test change…
dfed 72bb5e5
Get rid of ErrorHandler
dfed 5127024
couldNotReadKeychain -> .couldNotAccessKeychain
dfed eba3e95
Commit the error files
dfed c39612c
Make Objective-C bridging methods for accessing values with prompt re…
dfed 5165c3d
Update tests
dfed f6d5c6a
Fix warnings
dfed 4f57bcf
Get tests running
dfed 80e0a71
Simpler flow
dfed aece8fb
Swift 5 updates
dfed 61fcab5
Update documentation
dfed 5ebe4ae
Update ValetTouchIDTest
dfed 10ebf88
Update README again
dfed d05479b
More swift 5
dfed a735f5b
More documentation
dfed b4c0f27
Remove incorrect documentation
dfed fe21e04
More documentation
dfed 882f376
Clean up @objc declarations
dfed a06ab81
Bring API in line with Apple's naming guidelines
dfed 430ff5b
Fix post-rebase
dfed b607e8b
Fix test
dfed 73ba03f
Merge branch 'develop--4.0' into dfed--throws-api
dfed 4694bc3
Merge branch 'develop--4.0' into dfed--throws-api
dfed a02cc30
Fix warning introduced by merge
dfed f8f33d0
Fix merge conflict test break
dfed 670f5fb
Merge branch 'develop--4.0' into dfed--throws-api
dfed 17d1eb2
Merge branch 'develop--4.0' into dfed--throws-api
dfed 2fbba9d
Modernize doc comments
dfed 66efc3f
Do not capitalize Key in documentation
dfed 82f45a0
Single parameter should not have the Parameters heading in docs
dfed 4ed9fa3
Add missing parameters to documentation
dfed b468699
Key -> key
dfed 81bbc25
Merge branch 'dfed--cleanup' into dfed--throws-api
dfed bbb2ed9
Fixup missing Parameters
dfed 99ee957
Merge branch 'dfed--cleanup' into dfed--throws-api
dfed 74b7822
whitespace and merge conflicts
dfed d79aacc
Merge branch 'dfed--cleanup' into dfed--throws-api
dfed 688bcb0
whitespace
dfed a86a468
Merge branch 'develop--4.0' into dfed--throws-api
dfed 74d2e8d
Merge branch 'develop--4.0' into dfed--throws-api
dfed 5893973
Fix whitespace
dfed ee56d51
Fail test on setup failure
dfed e0f33e1
Update comment
dfed a6fcdfb
Revert comment
dfed 0032221
Update whitespace
dfed 2f0d5cc
containsObject(forKey should throw in Swift
dfed 873cda1
Use nullability rather than throws to indicate 'item not found'
dfed dab04b4
Revert "Use nullability rather than throws to indicate 'item not found'"
dfed e54bb22
ADd missing Returns comments
dfed 630dd66
Introduce Throws doc comment
dfed 455e401
Catch closer to the source
dfed 42fa0a6
Fixup whitespace
dfed 42f3925
try? less in tests
dfed adcea82
If deleting items throws, then we should surface the failure.
dfed a7d5f09
Merge branch 'develop--4.0' into dfed--throws-api
dfed bb12909
Remove runtime assert, since we will throw the error anyways
dfed 1a6fade
Update Mac tests to use try on containsObject
dfed f718dc0
Add description to KeychainError
dfed 265e3c8
Fix macOS tests after throwing on removeAllObjects in setUp
dfed b1d205b
Increase test coverage of error files
dfed cadeb20
vanillaValet -> valet, anotherFlavor -> iCloudValet
dfed f78e319
Rename test
dfed 7bf227d
Revert comment change
dfed e60f5d3
Update whitespace
dfed 2b20666
Update comment formatting
dfed eca2b7d
Revert "vanillaValet -> valet, anotherFlavor -> iCloudValet"
dfed 93b4cef
Use permutation valet rather than vanillaValet multiple times
dfed 697e53f
Indentation and test separation
dfed 883ace0
Add comment re why we're checking for errSecInteractionNotAllowed
dfed efeced6
Better comment formatting
dfed 249188f
> 0 -> not isEmpty
dfed e7023c9
Use Throws rather than Note
dfed d1b2d80
whitespace
dfed 029927a
Add final to test class
dfed 40344ef
Make test class final
dfed 4e9eb8b
Expand default
dfed fd5feba
Rename internal containsObject methods to performCopy
dfed File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thoughts on whether we should show the error being ignored? Both here and in the Objective-C code below. Previously we were discarding the return type, so the current sample-code maintains the prior behavior.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems reasonable to keep the example simple. The presence of
try?implies that there is an error case that could (should) be handled in real usage.