Skip to content

Bump version to 0.5.0, update dependencies, and pin Rust 1.91 in CI#12

Merged
soulgarden merged 3 commits intomainfrom
0.5.0
Dec 28, 2025
Merged

Bump version to 0.5.0, update dependencies, and pin Rust 1.91 in CI#12
soulgarden merged 3 commits intomainfrom
0.5.0

Conversation

@soulgarden
Copy link
Owner

@soulgarden soulgarden commented Dec 28, 2025

  • Update all crate dependencies to latest compatible versions (46 packages)
  • Sync package version to 0.5.0 across Cargo.toml, VERSION, and Helm charts
  • Pin CI workflow to Rust 1.91 for consistent builds

Summary by CodeRabbit

  • Chores

    • Version bumped to 0.5.0 and chart/image tag updated.
    • Deployment defaults updated.
  • Build/CI

    • CI reworked into separate lint, test, and conditional build/publish stages with push and PR triggers.
    • Added formatting and lint checks, aligned Rust toolchain, improved caching, and conditional Docker publish.
  • Build Tooling

    • New lint-fix target and adjusted lint/dev check workflow.
  • Tests

    • Tests refined for clearer assertions and simplified control flow.

✏️ Tip: You can customize this high-level summary in your review settings.

- Update all crate dependencies to latest compatible versions (46 packages)
- Sync package version to 0.5.0 across Cargo.toml, VERSION, and Helm charts
- Pin CI workflow to Rust 1.91 for consistent builds
@coderabbitai
Copy link

coderabbitai bot commented Dec 28, 2025

📝 Walkthrough

Walkthrough

Split CI into lint/test/build with Rust 1.91 pinning and conditional Docker push; bump project and Helm versions to 0.5.0; add lint_fix Makefile target; small test refactors/simplifications in a few Rust modules. No API or runtime logic changes.

Changes

Cohort / File(s) Summary
CI/CD workflow
.github/workflows/main.yml
Rework single build job into separate lint, test, and conditional build stages; pin Rust toolchain to 1.91; add push/pull_request triggers, explicit permissions, DO_PUSH guard, updated cargo cache key, and conditional Docker login/build/push flows for PR vs main.
Version bumps & Helm
Cargo.toml, VERSION, helm/logfowd2/Chart.yaml, helm/logfowd2/values.yaml
Bump package, repo VERSION, Helm chart version/appVersion, and default image tag to 0.5.0.
Build tooling / Makefile
Makefile
Add lint_fix target; change lint to cargo clippy --all-targets -- -D warnings; update .PHONY and dev_check dependency to use lint_fix.
Rust tests / small refactors
src/domain/state.rs, src/infrastructure/metrics/server.rs, src/transport/bridge/event_bridge.rs, src/watcher.rs
Minor test refactors and assertion/style simplifications (combine conditionals, replace equality assertions with boolean checks, clarify/no-op assertion comments). No production code signature or behavior changes.

Sequence Diagram(s)

sequenceDiagram
    autonumber
    actor Developer
    participant GitHub as "GitHub (Push/PR)"
    participant Runner as "Actions Runner"
    participant Cache as "Cargo Cache"
    participant Registry as "Docker Registry"

    Developer->>GitHub: Push branch or open PR
    GitHub->>Runner: Start workflow (push / pull_request)
    Runner->>Cache: Restore cargo cache (includes Rust version in key)
    Runner->>Runner: Setup Rust 1.91
    Runner->>Runner: Lint (cargo fmt, clippy)
    Runner->>Runner: Test (cargo test)
    alt lint & test succeed
        Runner->>Runner: Build job (depends on lint & test)
        alt DO_PUSH=true and branch == main
            Runner->>Runner: Docker login (conditional)
            Runner->>Runner: Docker build (main tags)
            Runner->>Registry: Push image
        else PR or DO_PUSH=false
            Runner->>Runner: Docker build (PR/branch tags, push disabled)
        end
    else lint/test fail
        Runner->>Developer: Workflow fails (logs)
    end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰
I hopped through manifests, tidy and spry,
Bumped versions up and watched CI fly.
Lint then test, build on cue,
Rust 1.91 — clean and new.
I nibble a carrot; the pipeline says woohoo!

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main changes: version bumps to 0.5.0, Rust 1.91 pinning in CI, and mentions updates to dependencies.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch 0.5.0

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 69f320b and 32614b4.

📒 Files selected for processing (6)
  • .github/workflows/main.yml
  • Makefile
  • src/domain/state.rs
  • src/infrastructure/metrics/server.rs
  • src/transport/bridge/event_bridge.rs
  • src/watcher.rs
✅ Files skipped from review due to trivial changes (1)
  • src/domain/state.rs
🧰 Additional context used
🧬 Code graph analysis (1)
src/infrastructure/metrics/server.rs (1)
src/infrastructure/metrics/collector.rs (1)
  • are_metrics_enabled (214-216)
🪛 checkmake (0.2.2)
Makefile

[warning] 3-3: Missing required phony target "all"

(minphony)


[warning] 3-3: Missing required phony target "clean"

(minphony)

🔇 Additional comments (10)
src/watcher.rs (2)

1591-1591: LGTM! Clear documentation of test intent.

The comment effectively documents that reaching this point confirms sync_files completed without panicking, replacing what was likely an always-true assertion.


1960-1960: LGTM! More idiomatic assertion.

Using !is_empty() is clearer and more idiomatic than len() >= 1, and aligns with Clippy's len_zero lint.

src/transport/bridge/event_bridge.rs (1)

392-397: LGTM! Clean use of let-chains for Rust 2024.

The refactor combines the timeout check with result.is_ok() using let-chains, reducing nesting while preserving the same logic. This is more concise and idiomatic.

src/infrastructure/metrics/server.rs (1)

149-162: LGTM! More idiomatic boolean assertions.

Refactoring from assert_eq!(..., false) to assert!(!...) is clearer and aligns with Clippy's bool_assert_comparison lint recommendation.

.github/workflows/main.yml (5)

3-9: LGTM! Good security and trigger configuration.

Adding pull_request triggers ensures PRs are validated before merge, and explicit contents: read permissions follow the principle of least privilege.


10-11: LGTM! Clean conditional logic for Docker push.

The DO_PUSH environment variable correctly restricts Docker Hub pushes to the main branch, preventing accidental pushes from PRs or feature branches.


38-45: LGTM! Proper cache configuration.

The cache configuration correctly includes the Rust version (1.91) in the key, ensuring cache invalidation when the toolchain changes.


50-92: LGTM! Well-structured conditional build pipeline.

The build job properly depends on lint and test, and the conditional Docker operations cleanly separate PR validation (build-only) from production releases (build and push to Docker Hub). The different caching strategies (GHA cache for PRs, registry cache for main) are appropriate for each use case.


14-29: Rust 1.91 version is correct and intentional.

The project's use of Rust 1.91 is documented in the README as the minimum required version for Rust 2024 edition support (confirmed in Cargo.toml and rust-toolchain.toml). The lint job structure is well-organized with separate formatting and Clippy checks, and the Rust toolchain version is appropriately pinned.

Makefile (1)

12-16: LGTM! Excellent separation of lint check vs. auto-fix.

Splitting lint (check-only for CI) from lint_fix (auto-fix for local development) is a best practice. This aligns perfectly with the CI workflow changes where the lint job uses make lint to validate code without modifying it, while dev_check uses lint_fix to automatically fix issues during local development.

Also applies to: 36-36


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

- Split monolithic build job into lint, test, and build stages
- Add pull_request trigger for PR validation
- Docker push only on main branch (DO_PUSH condition)
- Use GitHub Actions cache (type=gha) for non-main builds
- Add explicit permissions (contents: read)
- Improve clippy check with --all-targets flag
- Include Rust version in cache key for consistency
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
.github/workflows/main.yml (1)

14-29: Consider updating to Rust 1.92.0, the latest stable release (December 11, 2025).

The lint job correctly runs format checks and Clippy with strict warnings. The --all-targets flag is good practice to lint tests and examples. However, the workflow is pinned to Rust 1.91, which is one release behind. Update to 1.92.0 unless the earlier version is pinned intentionally for reproducibility.

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9cb8993 and 69f320b.

📒 Files selected for processing (1)
  • .github/workflows/main.yml
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: test
  • GitHub Check: test
🔇 Additional comments (4)
.github/workflows/main.yml (4)

3-5: LGTM! Good addition of PR validation.

Adding the pull_request trigger enables the workflow to run on PRs, providing early feedback before merging.


7-8: LGTM! Good security practice.

Explicit contents: read permission follows the principle of least privilege. Read-only access is sufficient for this workflow.


10-11: LGTM! Conditional push logic is sound.

The DO_PUSH variable correctly evaluates to 'true' only for pushes to the main branch, preventing Docker pushes from PRs and feature branches.


50-92: LGTM! Well-structured conditional build strategy.

The build job is properly gated by lint and test completion. The dual-build approach is excellent:

  • PR/branch builds: Use GitHub Actions cache and skip push, providing fast validation without Docker Hub credentials
  • Main builds: Push to registry and use registry-based caching for production images

The conditional Docker login (line 61) prevents unnecessary authentication on PRs, improving security and efficiency.

Comment on lines +47 to +48
- name: Run tests
run: cargo test -- --test-threads=1
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Search for any comments or documentation about test-threads, single-threaded tests, or concurrency issues
rg -n -C3 'test-threads|single.?thread|sequential.?test|concurrency|shared.?state' --type=rust --type=md --type=toml --type=yaml

Repository: soulgarden/logfowd2

Length of output: 984


🏁 Script executed:

#!/bin/bash
# View the complete workflow file to see if there are comments explaining the test configuration
cat -n .github/workflows/main.yml | head -60

Repository: soulgarden/logfowd2

Length of output: 1737


🏁 Script executed:

#!/bin/bash
# Find all test files and check their structure
fd -e rs -path '*/test*' -o -path '*/tests/*' | head -20

Repository: soulgarden/logfowd2

Length of output: 231


🏁 Script executed:

#!/bin/bash
# Find test files in the repository
find . -name "*.rs" -type f | grep -E "(test|spec)" | head -20

Repository: soulgarden/logfowd2

Length of output: 89


🏁 Script executed:

#!/bin/bash
# Search for #[test] attributes and test functions to understand test structure
rg '#\[test\]|mod tests|#\[tokio::test\]' -A 5 --type rust

Repository: soulgarden/logfowd2

Length of output: 50376


🏁 Script executed:

#!/bin/bash
# Look for static variables, mutexes, or file I/O in tests that might indicate shared state
rg 'static|mutex|Mutex|File|fs::|shared|LOCK' --type rust -C 2 | head -100

Repository: soulgarden/logfowd2

Length of output: 5569


🏁 Script executed:

#!/bin/bash
# Check the integration_tests.rs file for any comments about test execution
head -50 src/integration_tests.rs

Repository: soulgarden/logfowd2

Length of output: 1274


🏁 Script executed:

#!/bin/bash
# Search for any TODO, FIXME, or comments that might explain test-threads requirement
rg -i 'todo|fixme|test.*thread|serial|race' src/integration_tests.rs

Repository: soulgarden/logfowd2

Length of output: 295


Verify and consider removing the --test-threads=1 flag to improve CI performance.

The --test-threads=1 flag forces sequential test execution, which significantly slows down the CI pipeline. Testing the codebase reveals:

  • No comments or documentation explaining why single-threaded execution is required
  • Tests use #[tokio::test] and are designed for concurrent execution
  • Multiple tests explicitly verify concurrent behavior (e.g., test_concurrent_shutdown_components, test_concurrent_producers_consumers)
  • The application itself prioritizes concurrency optimization ("Lock Contention Minimization", "bounded concurrency")
  • No global state, static variables, or resource conflicts that would mandate serialization

This flag should be removed unless there is a specific reason for it.

🤖 Prompt for AI Agents
.github/workflows/main.yml lines 47-48: The workflow forces tests to run
single-threaded via --test-threads=1 which slows CI; remove that flag to allow
parallel test execution unless a known test requires serialization. Edit the Run
tests step to run `cargo test` without the `--test-threads=1` argument, run the
test suite locally/CI to confirm no flakiness, and if any failing tests depend
on single-threading, document the specific test(s) and add targeted fixes or
explicit per-test synchronization instead of a global flag.

- Split Makefile lint into lint (check) and lint_fix (auto-fix)
- Update dev_check to use lint_fix for convenience
- CI workflow now uses make lint for consistency
- Fix collapsible_if warnings using let chains (Rust 2024)
- Fix bool_assert_comparison: use assert!(!expr) instead of assert_eq!(expr, false)
- Fix assertions_on_constants: remove useless assert!(true)
- Fix len_zero: use !is_empty() instead of len() >= 1
@soulgarden soulgarden merged commit 90ca8eb into main Dec 28, 2025
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant