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

Switch to monorepo #80

Merged
merged 43 commits into from
Jul 14, 2023
Merged

Switch to monorepo #80

merged 43 commits into from
Jul 14, 2023

Conversation

saleel
Copy link
Member

@saleel saleel commented Jun 12, 2023

  • Switch to monorepo pattern with yarn workspace
  • Move twitter specific circuit/contracts to separate package
  • Add tests for email verifier circuit
  • Add tests for twitter circuit
  • Add helpers library with reusable utils

@saleel saleel requested a review from Divide-By-0 June 26, 2023 15:37
@saleel saleel changed the title [DRAFT] Switch to monorepo Switch to monorepo Jun 26, 2023
@saleel saleel marked this pull request as ready for review June 26, 2023 15:38
.gitignore Outdated
src/contracts/out/
src/contracts/lib/
src/contracts/broadcast
pacakges/contracts/out/
Copy link
Member

Choose a reason for hiding this comment

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

typo on packages

Copy link
Member

Choose a reason for hiding this comment

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

wait so where did this file go? is there an updated readme for how to compile/run?

Copy link
Member Author

@saleel saleel Jul 5, 2023

Choose a reason for hiding this comment

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

The scripts folder was moved to packages/twitter-verifier-circuits/scripts along with the circom files for twitter.
I have to update the paths in Readme. I will do it if the structure makes sense.

@Divide-By-0
Copy link
Member

Cool -- some minor merge conflicts but other than that, code looks fine to me. Would be great to update the readme so its up to date with the new structure!

I don't have enough experience with npm package structures so willing to trust your experience here on that.

@saleel
Copy link
Member Author

saleel commented Jul 6, 2023

Yea, I will fix the conflicts and update the Readme. Some existing stuff in the Readme was also updated if I remember correctly.
Structure wise this is good IMO. The main change was splitting the Twitter circuit in to two - new tests work well but wanted to be sure I have not missed anything.

Another thing that could be improved is the helper package. There are many helper functions that could be cleaned/refactored for better consumption. We can work on that progressively to make this a proper SDK.

@gitguardian
Copy link

gitguardian bot commented Jul 10, 2023

⚠️ GitGuardian has uncovered 2 secrets following the scan of your pull request.

Please consider investigating the findings and remediating the incidents. Failure to do so may lead to compromising the associated services or software components.

🔎 Detected hardcoded secrets in your pull request
GitGuardian id Secret Commit Filename
7202475 Generic High Entropy Secret 58864c8 packages/twitter-verifier-app/index.jsx View secret
7202475 Generic High Entropy Secret bf7967c packages/twitter-verifier-app/index.jsx View secret
🛠 Guidelines to remediate hardcoded secrets
  1. Understand the implications of revoking this secret by investigating where it is used in your code.
  2. Replace and store your secrets safely. Learn here the best practices.
  3. Revoke and rotate these secrets.
  4. If possible, rewrite git history. Rewriting git history is not a trivial act. You might completely break other contributing developers' workflow and you risk accidentally deleting legitimate data.

To avoid such incidents in the future consider


🦉 GitGuardian detects secrets in your source code to help developers and security teams secure the modern development process. You are seeing this because you or someone else with access to this repository has authorized GitGuardian to scan your pull request.

Our GitHub checks need improvements? Share your feedbacks!

@saleel saleel force-pushed the yarn-workspace branch 3 times, most recently from 4133721 to 44a30f7 Compare July 10, 2023 06:24
@Divide-By-0 Divide-By-0 merged commit 6d89b0d into main Jul 14, 2023
@saleel saleel deleted the yarn-workspace branch August 21, 2023 19:47
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