Skip to content

chore: Use the idiomatic way of create a default for Config#150

Merged
doubleailes merged 1 commit intomainfrom
config_default
May 3, 2025
Merged

chore: Use the idiomatic way of create a default for Config#150
doubleailes merged 1 commit intomainfrom
config_default

Conversation

@doubleailes
Copy link
Copy Markdown
Owner

@doubleailes doubleailes commented May 3, 2025

PR Type

Enhancement, Documentation


Description

  • Refactored Config to implement Rust's Default trait idiomatically

    • Moved default config logic from default_config() to Default::default()
    • Updated all usages from Config::default_config() to Config::default()
  • Updated documentation and code examples to use Config::default()

  • Adjusted tests to use the new default config instantiation


Changes walkthrough 📝

Relevant files
Enhancement
1 files
config.rs
Refactor Config to implement Default trait idiomatically 
+6/-4     
Documentation
6 files
lib.rs
Update code examples to use Config::default()                       
+1/-1     
rpc_client.rs
Update RpcClient docs and code to use Config::default()   
+11/-11 
rpc_service.rs
Update RpcService docs and code to use Config::default() 
+9/-9     
rpc_task.rs
Update RpcTask docs to use Config::default()                         
+2/-2     
README.md
Update README config example to use Config::default()       
+1/-1     
quick-start.md
Update quick start guide to use Config::default()               
+1/-1     

Need help?
  • Type /help how to ... in the comments thread for any questions about Qodo Merge usage.
  • Check out the documentation for more information.
  • @doubleailes doubleailes requested a review from Copilot May 3, 2025 08:46
    Copy link
    Copy Markdown

    Copilot AI left a comment

    Choose a reason for hiding this comment

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

    Pull Request Overview

    This PR updates the codebase to use the idiomatic Rust Default trait for creating a Config instance by replacing calls to Config::default_config() with Config::default().

    • Update of inline examples and documentation in multiple files (rpc_task.rs, rpc_service.rs, rpc_client.rs, lib.rs, quick-start.md, README.md)
    • Implementation change in config.rs: converting the custom default_config() method into a Default trait implementation

    Reviewed Changes

    Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.

    Show a summary per file
    File Description
    girolle/src/rpc_task.rs Updated documentation example to use Config::default()
    girolle/src/rpc_service.rs Updated documentation examples to use Config::default()
    girolle/src/rpc_client.rs Updated documentation examples to use Config::default()
    girolle/src/lib.rs Updated documentation example to use Config::default()
    girolle/src/config.rs Switched from default_config() to a Default trait implementation
    docs/content/docs/getting-started/quick-start.md Updated code sample to use Config::default()
    README.md Updated code sample to use Config::default()

    @qodo-code-review
    Copy link
    Copy Markdown

    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 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Naming Convention

    The constant AMQP_URI uses uppercase naming which doesn't follow Rust's naming convention for struct fields. Consider renaming to snake_case (e.g., amqp_uri) for consistency with other fields.

    AMQP_URI: Some("amqp://guest:guest@localhost/".to_string()),
    web_server_address: Some("".to_string()),

    @qodo-code-review
    Copy link
    Copy Markdown

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

    PR Code Suggestions ✨

    No code suggestions found for the PR.

    @doubleailes doubleailes merged commit 8758aef into main May 3, 2025
    5 checks passed
    @doubleailes doubleailes deleted the config_default branch May 3, 2025 08:52
    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.

    2 participants