-
Notifications
You must be signed in to change notification settings - Fork 228
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
Add CLI Foundry Template (cont.) #905
Conversation
v1.9.2
…into new_branch
v4.0.3
v4.0.3
v4.0.3
v4.0.3
v4.0.3
…ocs on integrate w/ boilerplate
Hey @jimmychu0807! I will review the PR this week. |
Thank you very much for creating this PR and following up @jimmychu0807 🙏 |
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.
Thank you so much for this PR @jimmychu0807 🙏
Since the Semaphore templates are configured to use yarn
, could you add a .yarnrc.yml
file and add the lines related to npm and yarn to the .gitignore
file: https://github.com/semaphore-protocol/semaphore/blob/main/packages/cli-template-contracts-hardhat/.gitignore
It would also be nice to have a yarn release in the template like in the others: https://github.com/semaphore-protocol/semaphore/tree/main/packages/cli-template-contracts-hardhat/.yarn/releases
Not all the tests are passing successfully for me, are all the tests passing successfully for you?
@vplasencia, thanks for the review.
hmm... I see. I wonder how do you run the test? The following is my screenshot: p.s. I have updated the gitignore and yarnrc file. p.p.s. I believe in this monorepo setting, the |
Hey @jimmychu0807!
One of my tests was failing because I had to run
Yes, the templates should be tested outside because they will be independent projects. This will be executed to create a new project using a template: https://github.com/semaphore-protocol/semaphore/blob/main/packages/cli/src/index.ts#L54 |
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.
It would be nice to add the Foundry template to the CLI so that developers can use the template right after it's ready. You can do this by adding:
{
value: "contracts-foundry",
name: "Foundry"
}
to the supportedTemplates
here: https://github.com/semaphore-protocol/semaphore/blob/main/packages/cli/src/index.ts#L23-L36
ah, I see the context now. Let me update it accordingly. |
@vplasencia Updated the packages/cli and remapping path. I wonder how do you test the packages/cli locally? I run the command |
Hey @jimmychu0807 thank you very much for all the updates.
To test the new template you can just copy/paste the |
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.
Thank you very much for the great work @jimmychu0807 🚀
Description
Completing #854
node_modules
instead of git submodules in this repo.Makefile
.This is a draft PR, but please review the updated approach and see if it is okay.
A few tasks left:
Related Issue(s)
closes #185
Other information
cc @cedoor @vplasencia @sripwoud
cc @timou0911 @csiejimmyliu
I thought I could continue building on top of #854, but end up opening a new PR. Let me know if there is a way / how to do it if this is important enough.
Checklist
yarn format
andyarn lint
without getting any errors