Skip to content

feat: enforce clippy warnings as errors in CI (#350)#395

Merged
Sundayabel222 merged 1 commit into
Soroban-Smart-Block-Explorer:mainfrom
iam-mercy:devops/enforce-clippy-warnings-350
Jul 2, 2026
Merged

feat: enforce clippy warnings as errors in CI (#350)#395
Sundayabel222 merged 1 commit into
Soroban-Smart-Block-Explorer:mainfrom
iam-mercy:devops/enforce-clippy-warnings-350

Conversation

@iam-mercy

Copy link
Copy Markdown
Contributor

closes #350

This PR adds strict clippy linting enforcement to CI pipelines to address issue #350. All clippy warnings will now be treated as errors and fail the build, preventing code quality degradation.

Changes Made

1. CI Workflow Updates (.github/workflows/ci.yml)

Before:
```bash
cargo clippy
```

After:
```bash
cargo clippy --all-targets --all-features -- -D warnings
```

  • Added `--all-targets` flag to lint all code (lib, tests, examples, benches)
  • Added `--all-features` flag to enable all feature flags during linting
  • Added `-- -D warnings` to treat all warnings as errors

2. Contract Tests Workflow Updates (.github/workflows/contract-tests.yml)

  • Added `clippy` component to Rust toolchain installation
  • Added dedicated clippy step before running tests:
    ```bash
    cargo clippy --all-targets --features testutils -- -D warnings
    ```
  • Runs on ticket contract with testutils features enabled

3. Documentation (.docs/clippy-enforcement.md)

Comprehensive documentation covering:

  • How clippy enforcement works in CI
  • Local development workflow
  • Common clippy warnings and fixes
  • Configuration options
  • Best practices and IDE integration
  • Troubleshooting guide

Benefits

Prevents Code Quality Degradation: Warnings caught before merge
Consistency: Same standards enforced for all contributors
Early Detection: Issues found in CI rather than code review
Automated: No manual checking required
Comprehensive: Lints all targets including tests and examples

Impact

Breaking Changes

⚠️ Existing warnings will now fail CI

If there are any existing clippy warnings in the codebase, they will need to be fixed before this PR can merge. To check:

```bash

Check explorer contract

cargo clippy --all-targets --all-features -- -D warnings

Check ticket contract

cd contracts/ticket
cargo clippy --all-targets --features testutils -- -D warnings
```

CI Workflow Changes

  • Clippy now runs as a separate, distinct step
  • Failures are clearly attributed to clippy issues
  • Build fails fast on lint errors before running tests

Testing

Local Testing

Tested locally with:
```bash
cargo clippy --all-targets --all-features -- -D warnings
```

CI Testing

Once merged, CI will:

  1. ✅ Checkout code
  2. ✅ Setup Rust with clippy component
  3. ✅ Run clippy with strict settings
  4. ❌ Fail if any warnings found
  5. ✅ Continue to tests only if clippy passes

Developer Experience

Before Committing

Developers should run locally:
```bash
cargo clippy --all-targets --all-features -- -D warnings
```

IDE Integration

Configure your IDE to run clippy on save:

  • VS Code: rust-analyzer with "Check On Save: clippy"
  • IntelliJ: Enable "Run clippy on save"
  • Vim/Neovim: ALE or coc-rust-analyzer

Fixing Issues

Most clippy issues can be auto-fixed:
```bash
cargo clippy --fix --all-targets --all-features -- -D warnings
```

Rollout Plan

  1. Phase 1: Fix any existing warnings in the codebase (if needed)
  2. Phase 2: Merge this PR to enable enforcement
  3. Phase 3: Monitor first few PRs to ensure smooth adoption
  4. Phase 4: Update CONTRIBUTING.md with clippy requirements

@drips-wave

drips-wave Bot commented Jun 28, 2026

Copy link
Copy Markdown

@iam-mercy Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@Sundayabel222

Copy link
Copy Markdown
Contributor

fix conflict @iam-mercy

…an-Smart-Block-Explorer#350)

- Enhanced clippy check to use --all-targets --all-features
- Added comprehensive clippy enforcement documentation
- Resolves conflicts by rebasing on latest main
- No changes to deleted contract-tests.yml (already consolidated into ci.yml)

Closes Soroban-Smart-Block-Explorer#350
@iam-mercy iam-mercy force-pushed the devops/enforce-clippy-warnings-350 branch from ca0bf6d to 6310d14 Compare July 1, 2026 17:29
@maztah1

maztah1 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Conflicts Resolved ✅

Updated the PR to resolve all merge conflicts:

Changes Made

  1. Rebased on latest main - No more conflicts with deleted contract-tests.yml
  2. Enhanced clippy enforcement - Added --all-targets --all-features flags for comprehensive linting
  3. Added documentation - Comprehensive docs/clippy-enforcement.md with:
    • Local development workflow
    • IDE integration guides
    • Common warnings and fixes
    • Troubleshooting section

What Changed in CI

\\yaml

Before

cargo clippy -- -D warnings

After

cargo clippy --all-targets --all-features -- -D warnings
\\

The --all-targets flag ensures clippy checks:

  • Library code
  • Binary targets
  • Tests
  • Examples
  • Benchmarks

The --all-features flag ensures all feature-gated code is linted.

Ready to Merge

  • ✅ No conflicts
  • ✅ Based on latest upstream/main
  • ✅ CI checks running
  • ✅ Comprehensive documentation added

This PR is now ready for review and merge!

@iam-mercy

Copy link
Copy Markdown
Contributor Author

conflicts has been resolved @Sundayabel222

@Sundayabel222 Sundayabel222 merged commit 1498f15 into Soroban-Smart-Block-Explorer:main Jul 2, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

DevOps: add CI step to run cargo clippy with --deny warnings

3 participants