Skip to content

Conversation

@Syati
Copy link
Owner

@Syati Syati commented Sep 9, 2025

This pull request updates the type signatures for the attributes and serialize_structured_value methods in both the implementation (lib/structured_params/params.rb) and the interface (sig/structured_params/params.rbs). The main change is to make the compact and symbolize keyword arguments optional, which improves flexibility and better matches Ruby conventions.

Type signature updates:

  • Made the compact and symbolize keyword arguments optional in the attributes method signature in both the implementation and the RBS file, allowing callers to omit these parameters if they want default behavior. [1] [2]
  • Updated the serialize_structured_value method signature to make the compact keyword argument optional in both the implementation and the RBS file. [1] [2]… in params.rb and params.rbs

Copilot AI review requested due to automatic review settings September 9, 2025 00:14
Copy link
Contributor

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 pull request updates type signatures for the attributes and serialize_structured_value methods to make keyword arguments optional, improving API flexibility and following Ruby conventions.

  • Made compact and symbolize keyword arguments optional with the ? prefix
  • Simplified type signatures by consolidating multiple overloads into fewer, more flexible signatures
  • Updated both implementation comments and RBS type definitions for consistency

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
sig/structured_params/params.rbs Updated RBS type signatures to use optional keyword arguments with ? prefix
lib/structured_params/params.rb Updated inline type comments to match the RBS changes

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Comment on lines 41 to 44
# : (?symbolize: true, ?compact: bool) -> Hash[Symbol, untyped]
# : (?symbolize: false, ?compact: bool) -> Hash[String, untyped]
def attributes: (?symbolize: true, ?compact: bool) -> Hash[Symbol, untyped]
| (?symbolize: false, ?compact: bool) -> Hash[String, untyped]
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

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

The type signature is inconsistent with the implementation. The RBS defines symbolize with a default of true, but the implementation in params.rb line 78 shows symbolize: false as the default. This mismatch could lead to confusion and type checking errors.

Suggested change
# : (?symbolize: true, ?compact: bool) -> Hash[Symbol, untyped]
# : (?symbolize: false, ?compact: bool) -> Hash[String, untyped]
def attributes: (?symbolize: true, ?compact: bool) -> Hash[Symbol, untyped]
| (?symbolize: false, ?compact: bool) -> Hash[String, untyped]
# : (?symbolize: false, ?compact: bool) -> Hash[String, untyped]
# : (?symbolize: true, ?compact: bool) -> Hash[Symbol, untyped]
def attributes: (?symbolize: false, ?compact: bool) -> Hash[String, untyped]
| (?symbolize: true, ?compact: bool) -> Hash[Symbol, untyped]

Copilot uses AI. Check for mistakes.
@Syati Syati merged commit 9946584 into main Sep 9, 2025
6 checks passed
@Syati Syati deleted the fix_rbs branch September 9, 2025 00:17
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