feat: add cairo-lint ci #238
Conversation
|
@okhaimie-dev is attempting to deploy a commit to the keep-starknet-strange Team on Vercel. A member of the Team first needs to authorize it. |
.github/workflows/check.yml
Outdated
| run: cargo install scarb-cairo-lint --git https://github.com/keep-starknet-strange/cairo-lint | ||
|
|
||
| - uses: asdf-vm/actions/install@v3 | ||
| - run: scarb cairo-lint |
There was a problem hiding this comment.
I was thinking the CI check should fail if there are existing Cairo-lint warnings. What do you think?
There was a problem hiding this comment.
I am not sure about this, there are some linting warning I don't think we might want to follow.
Example:
warning: Plugin diagnostic: Leaving `panic!` in the code is discouraged.
--> /home/runner/work/shinigami/shinigami/packages/engine/src/cond_stack.cairo:90:18
|
90 | _ => panic!("Invalid condition")
I think there are some valid panics, so if this type fails might not be the desired result
There was a problem hiding this comment.
I'll see if there is a way to ignore lints, but I think all the existing lints we want to follow. Even the panic one, we don't want any panic conditions ( and this branch should never be reached anyway ).
.github/workflows/check.yml
Outdated
|
|
||
| - uses: asdf-vm/actions/install@v3 | ||
| - run: scarb cairo-lint | ||
| - run: scarb cairo-lint --fix |
There was a problem hiding this comment.
Is this run needed? The changes wouldn't persist to the branch right?
There was a problem hiding this comment.
I was also thinking to remove the run: scarb cairo-lint --fix but wanted to here your feedback on this
Added cairo-lint to the ci pipeline