Skip to content

Remove redundant WREQ module definition in error.rs#5

Merged
0x676e67 merged 1 commit intoerrorfrom
copilot/sub-pr-4
Nov 18, 2025
Merged

Remove redundant WREQ module definition in error.rs#5
0x676e67 merged 1 commit intoerrorfrom
copilot/sub-pr-4

Conversation

Copy link

Copilot AI commented Nov 18, 2025

Addresses feedback on #4 where src/error.rs redundantly called define_module for the WREQ module already defined in lib.rs.

Changes

  • Removed redundant static: Deleted static WREQ: Lazy<RModule> that was redefining the module
  • Updated macro: Changed define_exception! to use const_get() to look up the existing module instead of defining it
  • Aligned function signature: Modified error::include() to accept gem_module parameter and return Result<(), MagnusError>, matching the pattern in http::include() and client::include()

Before:

static WREQ: Lazy<RModule> = Lazy::new(|ruby| ruby.define_module(crate::RUBY_MODULE_NAME).unwrap());

macro_rules! define_exception {
    ($name:ident, $ruby_name:literal, $parent:expr) => {
        static $name: Lazy<ExceptionClass> = Lazy::new(|ruby| {
            ruby.get_inner(&WREQ)
                .define_error($ruby_name, $parent(ruby))
                .unwrap()
        });
    };
}

pub fn include(ruby: &Ruby) { /* ... */ }

After:

macro_rules! define_exception {
    ($name:ident, $ruby_name:literal, $parent:expr) => {
        static $name: Lazy<ExceptionClass> = Lazy::new(|ruby| {
            let wreq_module: RModule = ruby.class_object().const_get(crate::RUBY_MODULE_NAME).unwrap();
            wreq_module.define_error($ruby_name, $parent(ruby)).unwrap()
        });
    };
}

pub fn include(ruby: &Ruby, _gem_module: &RModule) -> Result<(), MagnusError> { /* ... */ }

💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@0x676e67 0x676e67 marked this pull request as ready for review November 18, 2025 14:50
@0x676e67 0x676e67 merged commit 6c5513f into error Nov 18, 2025
1 check passed
Copilot AI changed the title [WIP] Address feedback on error/exception handler implementation Remove redundant WREQ module definition in error.rs Nov 18, 2025
Copilot AI requested a review from 0x676e67 November 18, 2025 15:02
0x676e67 added a commit that referenced this pull request Nov 18, 2025
* feat(error): add error/exception handler

* update

* fmt

* update

* Update src/error.rs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/error.rs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/error.rs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Initial plan (#5)

* Initial plan (#6)

* fmt

* fmt

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
0x676e67 added a commit that referenced this pull request Nov 19, 2025
* feat(error): add error/exception handler

* update

* fmt

* update

* Update src/error.rs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/error.rs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Update src/error.rs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Initial plan (#5)

* Initial plan (#6)

* fmt

* fmt

* fmt

---------

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
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.

2 participants