-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
[TT-1326] Update Solidty Foundry pipeline with Slither #13986
Conversation
7d0f304
to
4326696
Compare
product: | ||
# 98.5 is the aspirational code coverage once we migrate all tests to Foundry | ||
# product that have that minimum coverage of 98.5 in the matrix below are currently excluded from the check | ||
- name: automation |
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.
Is it possible to move this to the top of the file, and also add the info if products have a gas snapshot, and if they use forge fmt? That would be a good single source of truth for all CI in this file
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.
this is a really cool idea!
@@ -11,28 +11,55 @@ jobs: | |||
name: Detect changes | |||
runs-on: ubuntu-latest | |||
outputs: | |||
changes: ${{ steps.changes.outputs.src }} | |||
src_changes: ${{ steps.changes.outputs.src }} |
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.
I think it makes sense to check for changes in the folders per product. Right now we would be running coverage for all products if any single one changes a line. Ideally, CI only runs for affected products.
Any changes to vendor
or shared
would trigger a full run, don't think we can get around that
run: | | ||
forge fmt --check | ||
SOL_FILES="${{ needs.changes.outputs.sol_files }}" |
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 need this over the extremely simple check we had before? This is lightning fast
… an action, remove whitespaces
Quality Gate passedIssues Measures |
Running everything, when
shared
file changed: https://github.com/smartcontractkit/chainlink/actions/runs/10200971215Running only one product: https://github.com/smartcontractkit/chainlink/actions/runs/10201139248