-
Notifications
You must be signed in to change notification settings - Fork 0
feat: commit artifacts #106
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
Conversation
| @@ -1,7 +1,7 @@ | |||
| [profile.default] | |||
| solc = "0.8.23" | |||
| src = "src" | |||
| out = "out" | |||
| out = "artifacts" | |||
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.
To be consistent with Hardhat. This will prevent client repositories from having to change folder names to get the ABI, depending on the framework we use. This is an internal complexity for us.
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.
Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #106 +/- ##
=======================================
Coverage ? 83.78%
=======================================
Files ? 4
Lines ? 111
Branches ? 7
=======================================
Hits ? 93
Misses ? 17
Partials ? 1 ☔ View full report in Codecov by Sentry. |
… and add foundry_out_directory to slither config
…used configurations from config.json
zguesmi
left a comment
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.
Since we only have a small number of contracts, why not just commit their artifacts instead of committing a lot of unused artifacts?
I think something like this would be enough:
# Ignore all artifacts except some files.
!/artifacts/
/artifacts/**
!/artifacts/*IexecLayerZeroBridge.sol
!/artifacts/*IexecLayerZeroBridge.sol/*
!/artifacts/*RLCCrosschainToken.sol
!/artifacts/*RLCCrosschainToken.sol/*
!/artifacts/*RLCLiquidityUnifier.sol
!/artifacts/*RLCLiquidityUnifier.sol/*
|
Yeah it's true @zguesmi . You're right |
It really depends on where we'll be using these artifacts. If interfaces are not useful for Tenderly we can remove them at least for now. You can keep |
Objective
Improve traceability and consistency of build artifacts by committing them to the repo and adding automatic verification.
Main changes:
• Renamed build folder from
out/toartifactsfor consistency with Hardhat standards• Build artifacts are now tracked in Git (removed from .gitignore)
• Added a CI workflow to verify artifacts: runs forge build, compares results with committed artifacts, and blocks PRs if they are outdated.