Skip to content

[code ok] issue 441 remove passphrase table#577

Merged
tomholub merged 8 commits intomasterfrom
feature/issue-441-remove-passphrase-table
Oct 3, 2021
Merged

[code ok] issue 441 remove passphrase table#577
tomholub merged 8 commits intomasterfrom
feature/issue-441-remove-passphrase-table

Conversation

@sosnovsky
Copy link
Collaborator

@sosnovsky sosnovsky commented Sep 30, 2021

This PR removes PassPhraseObject model and adds optional passphrase field to KeyInfo class.

close #441


Tests:

  • Tests added or updated - I removed tests for methods which are not used anymore, and also added check for passphrase in testKeyInfoInit test

To be filled by reviewers

I have reviewed that this PR... (tick whichever items you personally focused on during this review):

  • addresses the issue it closes (if any)
  • code is readable and understandable
  • is accompanied with tests, or tests are not needed
  • is free of vulnerabilities
  • is documented clearly and usefully, or doesn't need documentation

@sosnovsky sosnovsky requested a review from tomholub September 30, 2021 13:51
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

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

Still need to go through the rest of the code (checked about half) but it's looking good. Left some comments.

@sosnovsky sosnovsky marked this pull request as draft September 30, 2021 20:44
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

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

Other than earlier comment about forgetting pass phrases on logout, the code looks good. Will now do manual testing.

@sosnovsky sosnovsky marked this pull request as ready for review October 1, 2021 11:00
Copy link
Collaborator

@tomholub tomholub left a comment

Choose a reason for hiding this comment

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

Looks good, waiting for updated xcode to try

@tomholub tomholub changed the title Feature/issue 441 remove passphrase table [code ok] issue 441 remove passphrase table Oct 2, 2021
@tomholub tomholub merged commit 81f8d4f into master Oct 3, 2021
@tomholub tomholub deleted the feature/issue-441-remove-passphrase-table branch October 3, 2021 07:49
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.

remove separate pass phrase table, add as a column on keys table

2 participants