-
Notifications
You must be signed in to change notification settings - Fork 7
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
feat: monorepo conversion #42
Conversation
If you're new to commit signing, there are different ways to set it up: Sign commits with
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. A couple of general comments:
- Please review the
.github/workflows/npm_publish.yml
job file that publishes to the npm registry on new version numbers. I think for this PR a change may not be necessary but please review it and confirm that it's the case. - As a organization requirement we request contributors to sign commits
package.json
Outdated
@@ -3,31 +3,13 @@ | |||
"version": "0.3.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are currently at version 0.3.1 so we need to bump this to 0.3.2 for example. Then please run Probably need to do the same for all packages individually.npm i
so the package-lock.json is also updated.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
by switching to pnpm we no longer have a package-lock
@@ -0,0 +1,274 @@ | |||
import { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The walletconnect provider has been updated after this PR was published, so you need to apply those changes in this file to avoid a regression
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated but please review that I didn't screw up the rebase
# Conflicts: # packages/tarijs/src/providers/walletconnect/index.ts
Description
In order to support both browser and nodejs environments we need to allow users to choose which packages to import.
The first step along that road is to structure the project as a monorepo of multiple packages.
At this time all the packages are required for tarijs, however in the future these requirements will be eliminated and instead passed as parameters based on which provider package is installed.
How Has This Been Tested?
Building the project
What process can a PR reviewer use to test or verify this change?
after installing proto run
moon tarijs:build
then navigate to packages/tarijs directory. you should have the build output in dist subdirectory. You should also run the integration testsBreaking Changes