Skip to content

issue #609 TypeScript-Core functionality to verify signatures - #625

Merged
tomholub merged 85 commits into
masterfrom
issue-609-verify-signatures
Oct 14, 2021
Merged

issue #609 TypeScript-Core functionality to verify signatures#625
tomholub merged 85 commits into
masterfrom
issue-609-verify-signatures

Conversation

@IvanPizhenko

@IvanPizhenkoIvanPizhenko commented Oct 6, 2021•
edited by tomholub
Loading

Copy link
Copy Markdown
Contributor

This PR refactors Typescript part in order to prepare to signature verification functionality.

close#609

Tests(delete all except exactly one):

  • Tests added or updated

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

tomholub
tomholub reviewed Oct 7, 2021
View reviewed changes

@tomholubtomholub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

looking good

Comment threadCore/source/core/pgp-msg.ts Outdated
Comment threadCore/source/core/pgp-msg.ts Outdated
tomholub
tomholub reviewed Oct 7, 2021
View reviewed changes

@tomholubtomholub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

checkpoint review, good

tomholub
tomholub reviewed Oct 7, 2021
View reviewed changes

@tomholubtomholub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

👍

@IvanPizhenko

IvanPizhenko commented Oct 7, 2021

Copy link
Copy Markdown
ContributorAuthor

So I think I've finished with changes in the code, now just need to test it all.

@IvanPizhenko
IvanPizhenko marked this pull request as ready for review October 12, 2021 20:56
@IvanPizhenko

IvanPizhenko commented Oct 12, 2021

Copy link
Copy Markdown
ContributorAuthor

@tomholub Typescript tests passed but Swift tests failed here.

@IvanPizhenko

IvanPizhenko commented Oct 12, 2021

Copy link
Copy Markdown
ContributorAuthor

Trying to rerun...

@IvanPizhenko

IvanPizhenko commented Oct 12, 2021

Copy link
Copy Markdown
ContributorAuthor

@tomholub Finally, ready for review, I think.

tomholub
tomholub reviewed Oct 13, 2021
View reviewed changes

@tomholubtomholub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks great - see comments

Comment threadCore/source/mobile-interface/endpoints.ts Outdated
Comment threadCore/source/mobile-interface/format-output.ts Outdated
Comment threadCore/source/mobile-interface/format-output.ts
@IvanPizhenko

IvanPizhenko commented Oct 13, 2021•
edited
Loading

Copy link
Copy Markdown
ContributorAuthor

@tomholub Fixed above notes. Swift test still failing.

tomholub
tomholub approved these changes Oct 14, 2021
View reviewed changes

@tomholubtomholub left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This looks good. Thanks! Since this only updates TS and TS tests pass, I'll force-merge this.

@tomholub

tomholub commented Oct 14, 2021

Copy link
Copy Markdown
Collaborator

This looks good. Thanks! Since this only updates TS and TS tests pass, I'll force-merge this.

Actually no - it could still affect the app. I'll set auto-merge, once it's updated from master in a way that passes tests, it should merge.

@tomholub
tomholub enabled auto-merge (squash) October 14, 2021 09:07
@tomholub
tomholub merged commit 2604979 into masterOct 14, 2021
@tomholub
tomholub deleted the issue-609-verify-signatures branch October 14, 2021 22:36
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tomholubtomholubtomholub approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

TypeScript-Core functionality to verify signatures

2 participants

@IvanPizhenko@tomholub