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.

Sign up for GitHub

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

Jump to bottom

Add API to withdraw verification requirement CryptoAPI.withdrawVerificationRequirement #4646

Merged
merged 2 commits into from
Jan 28, 2025
Merged

Add API to withdraw verification requirement CryptoAPI.withdrawVerificationRequirement #4646

merged 2 commits into from
Jan 28, 2025

Conversation

BillCarsonFr
Copy link
Member

Add API to withdraw verification requirement CryptoAPi.withdrawVerificationRequirement

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

@BillCarsonFr BillCarsonFr mentioned this pull request Jan 22, 2025
Merged
4 tasks
@BillCarsonFr BillCarsonFr added the T-Task Tasks for the team like planning label Jan 22, 2025
@BillCarsonFr BillCarsonFr force-pushed the valere/crypto_withdraw_verification_api branch from 3bacaca to 28d08c3 Compare January 22, 2025 13:54
@BillCarsonFr BillCarsonFr force-pushed the valere/crypto_withdraw_verification_api branch from 28d08c3 to 0745f53 Compare January 22, 2025 14:20
@BillCarsonFr BillCarsonFr marked this pull request as ready for review January 22, 2025 14:20
@BillCarsonFr BillCarsonFr requested a review from a team as a code owner January 22, 2025 14:20
@BillCarsonFr BillCarsonFr self-assigned this Jan 23, 2025
andybalaam
andybalaam approved these changes Jan 24, 2025
View reviewed changes
Copy link
Member

@andybalaam andybalaam left a comment

Choose a reason for hiding this comment

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

One optional comment, otherwise all good, thanks!

spec/unit/rust-crypto/rust-crypto.spec.ts Outdated
let rustCrypto: RustCrypto;
let olmMachine: Mocked<RustSdkCryptoJs.OlmMachine>;

beforeEach(() => {
Copy link
Member

@andybalaam andybalaam Jan 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I won't insist on it, but I have "opinions" on this style: setUp and tearDown considered harmful.

I personally would prefer a createRustCrypto function that is called at the beginning of each test. I feel it makes tests easier to read, and removes a risk of cross-test contamination.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done

@richvdh richvdh changed the title Add API to withdraw verification requirement CryptoAPi.withdrawVerificationRequirement Add API to withdraw verification requirement CryptoAPI.withdrawVerificationRequirement Jan 27, 2025
@richvdh richvdh removed their request for review January 27, 2025 12:04
@BillCarsonFr BillCarsonFr force-pushed the valere/crypto_withdraw_verification_api branch from 74b879e to 74a50f7 Compare January 28, 2025 10:35
@BillCarsonFr BillCarsonFr added this pull request to the merge queue Jan 28, 2025
Merged via the queue into develop with commit a2fd06b Jan 28, 2025
30 checks passed
@BillCarsonFr BillCarsonFr deleted the valere/crypto_withdraw_verification_api branch January 28, 2025 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andybalaam andybalaam andybalaam approved these changes

Assignees

@BillCarsonFr BillCarsonFr

Labels
T-Task Tasks for the team like planning
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@BillCarsonFr @andybalaam