-
Notifications
You must be signed in to change notification settings - Fork 346
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
chore: force the scratch buld on every push #735
Conversation
ce8a4c3
to
91c81b6
Compare
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.
LGTM. Curious why the rename from devnet -> local.
Feel like devnet was more explicit, but perhaps you prefer devnet to potentially mean something distributed?
- name: Install Foundry | ||
uses: foundry-rs/foundry-toolchain@v1 | ||
with: | ||
version: nightly |
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.
do we want to pin a version?
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.
pinned 1.2.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.
ahh, this doesn't work
only nightly is supported
per their docs;
Version to install, e.g. nightly or 1.0.0. Note: Foundry only has nightly builds for the time being.
All other versions 404
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.
Hmm iirc they keep monthly versions around. But latest is fine actually it’s actually good to make sure our stuff is deployable with latest foundry
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.
Realized that Deploy_From_Scratch
doesn't deploy the StrategyFactory
. Would be good to add to this PR
Edit: Seems like this will be done in a follow up PR? Please confirm and I'll approve :)
we'll update the script in a follow up PR (where I add logic to check that all contracts are deployed). Don't want multiple concerns in the same PR |
befdb75
to
45c64cb
Compare
|
Follow up: I'll check that all contracts in the repo are deployed, and fail if there are any that aren't included in the scratch build (to avoid losing sync with the repo).