Conversation
…fmt and LLVM linker installation steps
There was a problem hiding this comment.
Pull Request Overview
Adds CI workflow to run Cargo tests and formatting checks, with related code cleanup to ensure tests pass properly.
- Adds GitHub Actions workflow with format checking, testing, and building
- Fixes example binary path and removes unused imports
- Adds network test skipping mechanism for CI environment
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| .github/workflows/ci.yml | New CI workflow with format, test, and build jobs |
| crates/examples/Cargo.toml | Corrects binary path for view_function_example |
| crates/aptos-rust-sdk/src/tests/client/rest_api.rs | Adds network test skip guards |
| crates/aptos-rust-sdk/src/client/rest_api.rs | Adds network test skip guards and removes unused import |
| crates/aptos-rust-sdk-types/src/api_types/move_types.rs | Removes unused anyhow import |
| crates/aptos-crypto/src/unit_tests/bcs_test.rs | Formats long lines for readability |
| crates/aptos-crypto/src/encoding_type.rs | Formats long line for readability |
| crates/aptos-crypto-derive/src/lib.rs | Removes unused forbid unsafe_code attribute |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| - name: Add rustfmt component | ||
| run: rustup component add rustfmt |
There was a problem hiding this comment.
The rustfmt component is already being installed via the rust-toolchain action on line 22. This redundant step can be removed.
| - name: Add rustfmt component | |
| run: rustup component add rustfmt |
There was a problem hiding this comment.
I think, it is adding rustfmt and you don't need it, but it's pretty minor
0x-j
left a comment
There was a problem hiding this comment.
looks good, just need to address the comment from copilot
| - name: Add rustfmt component | ||
| run: rustup component add rustfmt |
There was a problem hiding this comment.
I think, it is adding rustfmt and you don't need it, but it's pretty minor
|
Let me give it a try. I remember that previously it would fail if I didn’t have it installed. I’ll test it now, please give me a moment. If there are any issues, I’ll revert the change. |
No description provided.