Skip to content

Conversation

@tungleduyxyz
Copy link
Contributor

@tungleduyxyz tungleduyxyz commented Jul 6, 2025

@tungleduyxyz tungleduyxyz force-pushed the fix_integration_test branch from b2cf04e to 13348c2 Compare July 6, 2025 02:22
@tungleduyxyz tungleduyxyz requested review from Copilot and pierre July 6, 2025 02:37
Copy link

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 addresses failures in integration tests by ensuring a default locale is applied and improving parameter encoding compatibility.

  • Adds a default 'en' locale fallback in invoice template retrieval to prevent nil-related errors.
  • Replaces filter_map with map + compact in HTTP parameter encoding for broader Ruby version support.

Reviewed Changes

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

File Description
lib/killbill_client/models/invoice.rb Introduce `locale
lib/killbill_client/api/net_http_adapter.rb Change parameter enumeration from filter_map to map + compact
Comments suppressed due to low confidence (4)

lib/killbill_client/models/invoice.rb:215

  • Consider updating the method’s documentation or docstring to mention that locale now defaults to 'en', so callers are aware of this behavior change.
          locale ||= 'en'

lib/killbill_client/models/invoice.rb:215

  • Add or update integration/unit tests to verify that omitting locale results in using 'en' and that the correct endpoint is called.
          locale ||= 'en'

lib/killbill_client/api/net_http_adapter.rb:160

  • Add tests for encode_params covering cases with nil values, arrays, and the withStackTrace flag to ensure encoding changes behave as expected.
          pairs = options[:params].map { |key, value|

lib/killbill_client/api/net_http_adapter.rb:160

  • [nitpick] Using map + compact incurs two iterations; consider using filter_map if targeting Ruby ≥ 2.7 to perform filtering and mapping in one pass.
          pairs = options[:params].map { |key, value|

@pierre pierre merged commit b76c256 into killbill:master Jul 6, 2025
6 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.

2 participants