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

fix: test/comment improvements from vault restival review #4815

Merged
merged 3 commits into from
Mar 11, 2022

Conversation

dtribble
Copy link
Member

Description

chore: improvements from restival review

  • added test for reallocate with unstaged seat
  • vault comment and naming improvements
  • improved error verification in tests

Security Considerations

None

Documentation Considerations

Improves internal documentation

Testing Considerations

None. Normal CI executes the additional tests.

refs: #4804

@dtribble dtribble requested a review from turadg March 10, 2022 17:53
Copy link
Member

@turadg turadg left a comment

Choose a reason for hiding this comment

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

LGTM. I assume you're squashing so the PR will go into the commit history. I think it shouldn't be 'fix:since it doesn't change any bugs (or even behavior).chore:` seems most appropriate.

{
message: 'Transfer during vault adjustment',
},
'adjust balances should have been rejected',
Copy link
Member

Choose a reason for hiding this comment

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

👍

@@ -636,6 +637,7 @@ export const makeInnerVault = (
updateUiState();
clientSeat.exit();

// TODO get rid of strings?
Copy link
Member

Choose a reason for hiding this comment

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

if you happen to make another commit on this branch, this could be a good issue for the TODO #4360

@dtribble dtribble enabled auto-merge (squash) March 10, 2022 18:23
* added test for reallocate with unstaged seat
* vault comment improvements
* improved error verification in tests
@dtribble dtribble merged commit 7d254f0 into master Mar 11, 2022
@dtribble dtribble deleted the vault_comments branch March 11, 2022 08:21
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