Skip to content

Conversation

@kevin-ip
Copy link
Contributor

@kevin-ip kevin-ip commented Mar 31, 2025

PR Type

Tests, Enhancement


Description

  • Added a GitHub Actions workflow for Rust client checks.

  • Configured caching for Rust dependencies to improve performance.

  • Included steps for running documentation and unit tests.

  • Enabled concurrency control to cancel in-progress runs for the same branch.


Changes walkthrough 📝

Relevant files
Tests
pr_checks_rs.yaml
Add GitHub Actions workflow for Rust client                           

.github/workflows/pr_checks_rs.yaml

  • Introduced a new GitHub Actions workflow for Rust client.
  • Configured dependency caching using Swatinem/rust-cache.
  • Added steps to run documentation and unit tests.
  • Enabled concurrency control for pull request checks.
  • +42/-0   

    Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @kevin-ip kevin-ip requested a review from a team March 31, 2025 17:34
    @qodo-merge-pro
    Copy link

    Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Test Coverage

    The workflow only runs doc tests and unit tests with --lib flag, which might miss integration tests or binary tests. Consider adding a step to run all tests with cargo test --workspace without flags.

    - name: Run doc and unit tests
      run: cargo test --doc --workspace && cargo test --lib --workspace
    Missing Format Check

    The job name mentions "Format" but there's no step that actually runs cargo fmt --check to verify code formatting.

    name: Format, Tests & Coverage

    @qodo-merge-pro
    Copy link

    qodo-merge-pro bot commented Mar 31, 2025

    Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Impact
    Security
    Use standard runners for security

    Consider using a standard GitHub-hosted runner instead of a self-hosted runner
    for better security. Self-hosted runners can be vulnerable to security risks
    when running code from external contributors in pull requests.

    .github/workflows/pr_checks_rs.yaml [19]

    -runs-on: self-hosted
    +runs-on: ubuntu-latest
    • Apply this suggestion
    Suggestion importance[1-10]: 8

    __

    Why: Using self-hosted runners for PR checks can pose significant security risks, especially when processing code from external contributors. GitHub-hosted runners provide an isolated, secure environment that automatically resets after each job.

    Medium
    General
    Split tests for better visibility

    Split the test commands into separate steps for better visibility of failures in
    the GitHub Actions UI. This makes it easier to identify which specific test type
    failed.

    .github/workflows/pr_checks_rs.yaml [41-42]

    -- name: Run doc and unit tests
    -  run: cargo test --doc --workspace && cargo test --lib --workspace
    +- name: Run documentation tests
    +  run: cargo test --doc --workspace
    +  
    +- name: Run unit tests
    +  run: cargo test --lib --workspace
    • Apply this suggestion
    Suggestion importance[1-10]: 6

    __

    Why: Splitting the test commands into separate steps improves workflow readability and debugging. When tests fail, it becomes immediately clear which type of test (documentation or unit) is failing, making troubleshooting more efficient.

    Low
    • Update

    @kevin-ip kevin-ip force-pushed the kevin/add-gha-for-rust-client branch from 2e650cd to 1d218ab Compare March 31, 2025 17:35
    alawrenc
    alawrenc previously approved these changes Mar 31, 2025
    @kevin-ip kevin-ip dismissed stale reviews from nikita-seedlabs and alawrenc via bf4c5e8 March 31, 2025 17:45
    @qodo-merge-pro
    Copy link

    qodo-merge-pro bot commented Apr 2, 2025

    Qodo Merge was enabled for this repository. To continue using it, please link your Git account with your Qodo account here.

    CI Feedback 🧐

    A test triggered by this PR failed. Here is an AI-generated analysis of the failure:

    Action: Tests

    Failed stage: [❌]

    Failure summary:

    The action failed because it was waiting for a runner to pick up the job but no runner was available
    or assigned. This is an infrastructure issue rather than a code problem. The job was defined in the
    workflow file pr_checks_rs.yaml but couldn't start execution.

    Relevant error logs:
    1:  Job defined at: fireflyprotocol/pro-sdk/.github/workflows/pr_checks_rs.yaml@refs/pull/47/merge
    2:  Waiting for a runner to pick up this job...
    

    @kevin-ip kevin-ip closed this Oct 9, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Projects

    None yet

    Development

    Successfully merging this pull request may close these issues.

    4 participants