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

Added new keychain path log #18

Merged

Conversation

omarzl
Copy link
Contributor

@omarzl omarzl commented May 23, 2024

This pull request adds a log after the keychain is created:

INFO: Running command line: bazel-bin/Sources/SignHereTool/sign-here create-keychain --keychain-name TestKeychain --keychain-password ***
Keychain created in the path: /Users/omarzl/Library/Keychains/TestKeychain-db

This small change can add these improvements:

  • There is a log that confirms to the user that the keychain was created
  • It is a sanity check that guarantees that they keychain was created by calling security list-keychains

Added tests for the new error types:

bazel test //Tests/...
//Tests/SignHereLibraryTests:SignHereUnitTests                  (cached) PASSED in 0.6s
    PASSED  .Tests/SignHereLibraryTests/SignHereUnitTests.test-runner.sh (0.0s)
Test cases: finished with 1 passing and 0 failing out of 1 test cases

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I got these changes, not sure if I should discard them

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks okay

Copy link
Collaborator

@tinder-maxwellelliott tinder-maxwellelliott left a comment

Choose a reason for hiding this comment

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

Thank you for adding this!

@tinder-maxwellelliott tinder-maxwellelliott merged commit 24ffc37 into Tinder:main May 23, 2024
4 checks passed
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