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

imp: cairo/scarb structure suggestions #78

Merged
merged 11 commits into from
Sep 11, 2024

Conversation

drspacemn
Copy link
Contributor

@drspacemn drspacemn commented Sep 11, 2024

Structure:

  • Separate out packages from top-level contracts
  • Include packages in virtual manifest
  • Remove redundant workspace definitions from workspace packages
  • Only define shared resources in top-level dependencies
  • Explicit tool version via .tool-versions file
  • Define all script testing via snforge
  • More explicit visibility

Personal Preferences:

  • define all modules in lib.cairo instead of breaking out small file imports
  • .workspace = true > = { workspace = true }
  • define package versions manually for more granular control
  • configure all sub modules under tests with #[cfg(test)] except for mod mocks in order to keep mocks segregated from business logic but still buildable via snforge

Recommendations:

  • Include mocks for all components and local testing infrastructure for the components via mocks

@drspacemn drspacemn changed the title cairo/scarb suggestions cairo/scarb/structure suggestions Sep 11, 2024
@Farhad-Shabani Farhad-Shabani changed the title cairo/scarb/structure suggestions imp: cairo/scarb structure suggestions Sep 11, 2024
Copy link
Member

@Farhad-Shabani Farhad-Shabani left a comment

Choose a reason for hiding this comment

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

All the points you mentioned sound great!

Just as for { workspace = true }, I kept it as-is to stay consistent with the other workspaces. Plus, I found it a bit cleaner to catch at a glance.

On the granular versioning side, since we're still early in development, we'll likely bump the version together for a while until things stabilize. But after, your suggestion definitely makes sense. I also got the meta starknet_ibc package back in.

Only remained local component testing. I opened up #79 to tackle it as a separate effort.

Thanks again for the awesome suggestions and the PR! ✨

@Farhad-Shabani Farhad-Shabani merged commit f5c4c27 into informalsystems:main Sep 11, 2024
6 checks passed
@justtry1512
Copy link

OK. I'M DOINT IT NOW.

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