-
Notifications
You must be signed in to change notification settings - Fork 13
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
fix: skip commit id and sync for signing keys #893
Conversation
key.startsWith(RegExp('private:|privatekey:|public:_'))) { | ||
(key.startsWith(RegExp( | ||
'private:|privatekey:|public:_|public:signing_publickey')) || | ||
key.contains(AT_SIGNING_PRIVATE_KEY))) { |
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.
Use of contains
seems potentially buggy. Why not an exact match?
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.
format of the key is <current_atsign>:signing_privatekey<current_atsign>
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.
key.startsWith(RegExp('@(?<sharedWith>.*):signing_privatekey@(?<sharedBy>.*)'))
is this fine?
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.
LGTM
@@ -341,8 +341,7 @@ class CommitLogKeyStore | |||
if ((RegExp(regex).hasMatch(atKey)) || | |||
atKey.contains(AT_ENCRYPTION_SHARED_KEY) || |
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.
Should we refactor this to:
match = _isRegexMatches() || _isSpecialKey();
_isSpecialKey contains checking for special keys
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.
yeah..sounds good.i will make the changes
…ion/at_server into sync_skipkeys_bug
@gkc addressed the comments. dismissing the review
- What I did
- How I did it
- How to verify it