Skip to content

refactor: reown #575

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
Merged
merged 15 commits into from
Oct 8, 2024
Merged

refactor: reown #575

merged 15 commits into from
Oct 8, 2024

Conversation

andreabadesso
Copy link
Contributor

@andreabadesso andreabadesso commented Sep 27, 2024 •
edited
Loading

Acceptance Criteria

  • We should use @reown/walletkit instead of @walletconnect/web3wallet
  • We should rename all instances of "wallet connect" to "reown"

Security Checklist

  • Make sure you do not include new dependencies in the project unless strictly necessary and do not include dev-dependencies as production ones. More dependencies increase the possibility of one of them being hijacked and affecting us.

@andreabadesso andreabadesso changed the base branch from master to hathorplay-base-release September 27, 2024 16:34
@andreabadesso andreabadesso self-assigned this Sep 27, 2024
@andreabadesso andreabadesso added the enhancement New feature or request label Sep 27, 2024
@andreabadesso andreabadesso force-pushed the refactor/reown branch 3 times, most recently from 55d9c61 to bb36a39 Compare October 1, 2024 16:02
@andreabadesso andreabadesso changed the base branch from hathorplay-base-release to hathorplay-base October 1, 2024 19:27
andreabadesso
andreabadesso commented Oct 1, 2024
View reviewed changes
package.json
"@walletconnect/react-native-compat": "^2.12.2",
"@walletconnect/web3wallet": "^1.14.1",
"@reown/walletkit": "1.1.0",
"@sentry/react-native": "5.31.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is unrelated to the walletkit refactor, but I had to upgrade it to make it work in the latest XCode

andreabadesso
andreabadesso commented Oct 1, 2024
View reviewed changes
package.json
"@walletconnect/web3wallet": "^1.14.1",
"@reown/walletkit": "1.1.0",
"@sentry/react-native": "5.31.0",
"@walletconnect/core": "2.17.0",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reown crashes on previous version

They didn't move it to the reown namespace yet

alexruzenhack
alexruzenhack approved these changes Oct 2, 2024
View reviewed changes
src/components/Reown/ApproveRejectModal.js Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

[note] I believe we shouldn't name our submodules with third party names. This feature could have received its own name to serve as an abstraction between our interpretation of the feature and what we are using underneath to implement it. This strategy can also make it a starting point for a generalization.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not renaming it in this PR though, it's outside of its scope

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem. Let address it later, but it would be nice to have a tracked issue.

pedroferreira1
pedroferreira1 reviewed Oct 4, 2024
View reviewed changes
@andreabadesso andreabadesso merged commit 28d1d4a into hathorplay-base Oct 8, 2024
@andreabadesso andreabadesso deleted the refactor/reown branch October 8, 2024 13:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pedroferreira1 pedroferreira1 pedroferreira1 approved these changes

@alexruzenhack alexruzenhack alexruzenhack approved these changes

Assignees

@andreabadesso andreabadesso

Labels
enhancement New feature or request
Projects
Hathor Network
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@andreabadesso @pedroferreira1 @alexruzenhack