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

chore: added more of personal_sign test cases #839

Conversation

JeneaVranceanu
Copy link
Collaborator

Summary of Changes

Based on conversations in Discord support channel I've added more personal_sign test cases as a redundancy.

Test Data or Screenshots

By submitting this pull request, you are confirming the following:
  • I have reviewed the Contribution Guidelines.
  • I have performed a self-review of my own code.
  • I have updated my repository to match the develop branch.
  • I have included test data or screenshots that prove my fix is effective or that my feature works.
  • I have checked that all tests work and swiftlint is not throwing any errors/warnings.

janndriessen
janndriessen previously approved these changes Jan 9, 2024
Copy link
Collaborator

@janndriessen janndriessen left a comment

Choose a reason for hiding this comment

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

Nice! LGTM - assuming those TODOs will be done some other time.

@JeneaVranceanu
Copy link
Collaborator Author

Nice! LGTM - assuming those TODOs will be done some other time.

I think I'll add them as a separate PR but based on this branch.

@JeneaVranceanu
Copy link
Collaborator Author

@janndriessen I've decided I'll push related changes regarding these TODOs into this PR.

@JeneaVranceanu
Copy link
Collaborator Author

@janndriessen I've decided I'll push related changes regarding these TODOs into this PR.

Sorry, there were too many changes and now there's a separate PR open that handles these TODOs and even more. See #841

Comment on lines +16 to +20
// TODO: add the following error message: Missing `attachedKeystoreManager`.
throw Web3Error.walletError
}
guard let ethAddresses = keystoreManager.addresses else {
// TODO: add the following error message: Missing attached keystore is empty.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These TODOs are resolved here: #841

@yaroslavyaroslav
Copy link
Collaborator

@JeneaVranceanu LGTM. It's required a second review though, ping me if you're wishing to force merge it.

@yaroslavyaroslav yaroslavyaroslav merged commit f730f02 into web3swift-team:develop-4.0 Feb 7, 2024
1 check passed
@JeneaVranceanu JeneaVranceanu deleted the chore/more-sign-tests branch February 7, 2024 20:55
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.

3 participants