Add cargo fmt and cargo clippy to CI workflows #2475
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Co-Authored-By: Jayant Krishnamurthy <jayant@dourolabs.xyz>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
3 Skipped Deployments
|
Co-Authored-By: Jayant Krishnamurthy <jayant@dourolabs.xyz>
| @@ -19,10 +20,22 @@ jobs: | |||
| - uses: actions/checkout@v3 | |||
|
|
|||
| - name: Download CLI | |||
| run: wget https://github.com/aptos-labs/aptos-core/releases/download/aptos-cli-v3.1.0/aptos-cli-3.1.0-Ubuntu-22.04-x86_64.zip | |||
| run: wget https://github.com/aptos-labs/aptos-core/releases/download/aptos-cli-v6.1.1/aptos-cli-6.1.1-Ubuntu-22.04-x86_64.zip | |||
There was a problem hiding this comment.
we had different versions in pre-commit and in this workflow.
|
|
||
| - name: Check Formatting | ||
| run: ./aptos move fmt | ||
| if: success() || failure() |
There was a problem hiding this comment.
this if condition runs the job regardless of what happened before, so you get feedback on formatting / lint / tests no matter what.
There was a problem hiding this comment.
Can we just remove the condition? Seems redundant...
There was a problem hiding this comment.
(aside) for posterity: this condition is false when the workflow is cancelled
| @@ -3,6 +3,7 @@ name: Test CosmWasm Contract | |||
| on: | |||
| pull_request: | |||
| paths: | |||
| - .github/workflows/ci-cosmwasm-contract.yml | |||
There was a problem hiding this comment.
I thought I already added all of these paths, but I guess not
There was a problem hiding this comment.
I don't think these need to be here, IIUC Github should automatically run workflows when their definition changes, but I could be wrong...
There was a problem hiding this comment.
it doesn't :( . I only added these after the workflows failed to trigger on the first push of this PR
| - apps/fortuna/** | ||
| push: | ||
| branches: [main] | ||
| jobs: | ||
| test: | ||
| runs-on: ubuntu-latest | ||
| defaults: | ||
| run: | ||
| working-directory: apps/fortuna |
There was a problem hiding this comment.
this working-directory field was missing on a few workflows... not sure how they worked before (perhaps they didnt?)
| @@ -11,6 +11,7 @@ module pyth::governance_action { | |||
| value: u8, | |||
| } | |||
|
|
|||
| #[lint::skip(unnecessary_numerical_extreme_comparison)] | |||
There was a problem hiding this comment.
I think something changed with Aptos again and there's a new lint error because CONTRACT_UPGRADE=0. The comparison is intentional though, so we want to ignore.
…itions Co-Authored-By: Jayant Krishnamurthy <jayant@dourolabs.xyz>
There was a problem hiding this comment.
I'm not sure I understand why we have all the success() || failure() conditions, is there any case where that would turn up false? Seems redundant, no?
I also think this could be majorly simplified using some of Github Actions' code organization features like reusable workflows and custom actions. There's a lot of duplication here which will make it a big pain to go back and update things in the future, especially if things need to shift in subtle ways between packages.
… if conditions" This reverts commit 37b0de9.
This PR adds cargo fmt and cargo clippy steps from .pre-commit-config.yaml to the relevant CI workflows. It also adds
if: success() || failure()conditions to ensure that formatting, clippy, and unit tests all run on every workflow trigger, regardless of whether previous steps fail.Link to Devin run: https://app.devin.ai/sessions/bece3c7eabb04acf9317107609d52113