Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

fix: Regional API domain processing#765

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Merged
manisha1997 merged 32 commits intomainfromregional-api-domain-processing
Nov 28, 2025

Conversation

@manisha1997
Copy link
Contributor

@manisha1997manisha1997 commentedNov 26, 2025•
edited
Loading

Fixes

Changes:

  1. Added deprecation warning
  2. Mapping of region edge
Screenshot 2025-11-26 at 11 19 21 PM

Checklist

  • I acknowledge that all my contributions will be made under the project's license
  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read theContribution Guidelines and my PR follows them
  • I have titled the PR appropriately
  • I have updated my branch with the main branch
  • I have added tests that prove my fix is effective or that my feature works
  • I have added the necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

If you have questions, please file asupport ticket, or create a GitHub Issue in this repository.

Copy link

CopilotAI 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 implements automatic edge mapping from region codes and deprecates theedge parameter in favor ofregion. When a region code (like 'ie1') is provided, the system now automatically maps it to the corresponding edge location (like 'dublin'). However, the implementation has several critical issues that need to be addressed.

Key Changes:

  • Added@@region_mappings hash to map region codes to edge locations
  • Removededge fromattr_accessor and added custom getter/setter with deprecation warnings
  • Modified initialization to automatically set edge based on region mapping
  • Updated tests to validate region-to-edge mapping and deprecation warnings

Reviewed changes

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

FileDescription
lib/twilio-ruby/base/client_base.rbAdds region-to-edge mapping hash, implements automatic edge assignment from region, adds deprecation warnings for edge getter/setter, removes edge from attr_accessor
spec/rest/client_spec.rbUpdates test to reflect that global edge config is now ignored, adds test for region-to-edge mapping with 'ie1' → 'dublin', adds test for edge setter deprecation warning

💡Add Copilot custom instructions for smarter, more guided reviews.Learn how to get started.

@password=password ||Twilio.auth_token
@region=region ||Twilio.region
@edge=Twilio.edge
@edge=!region.nil? ?@@region_mappings[region] :nil

Choose a reason for hiding this comment

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

Multiple issues with this edge initialization logic:

  1. IgnoresTwilio.edge completely: The old code was@edge = Twilio.edge, but now it's completely ignored. This is a breaking change that should either be documented or handled with a deprecation warning.

  2. Doesn't handle theTwilio.region fallback: Whenregion parameter isnil butTwilio.region is set (line 27), the mapping won't work because the check uses the parameterregion, not@region.

  3. Silent failure for unmapped regions: Returnsnil for invalid/unmapped regions without any warning.

Suggested fix:

@region=region ||Twilio.region@edge=if@region &&@@region_mappings[@region]@@region_mappings[@region]elsifTwilio.edgewarn'[DEPRECATION] Setting edge via Twilio.edge is deprecated. Use Twilio.region instead.'Twilio.edgeelsenilend
Suggested change
@edge= !region.nil? ?@@region_mappings[region] :nil
if@region &&@@region_mappings[@region]
@edge=@@region_mappings[@region]
elsifTwilio.edge
warn'[DEPRECATION] Setting edge via Twilio.edge is deprecated. Use Twilio.region instead.'
@edge=Twilio.edge
else
warn"[WARN] Region '#{@region}' is not mapped to an edge. @edge set to nil."unless@region.nil?
@edge=nil
end

Copilot uses AI. Check for mistakes.
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@password=password ||Twilio.auth_token
@region=region ||Twilio.region
@edge=Twilio.edge
@edge=!region.nil? ?@@region_mappings[region] :nil
Copy link
Contributor

Choose a reason for hiding this comment

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

+1

@sonarqubecloud
Copy link

Quality Gate FailedQuality Gate failed

Failed conditions
0.0% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

Copy link
Contributor

@tiwarishubham635tiwarishubham635 left a comment

Choose a reason for hiding this comment

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

LGTM!

@manisha1997manisha1997 merged commitbd17551 intomainNov 28, 2025
27 of 28 checks passed
@manisha1997manisha1997 deleted the regional-api-domain-processing branchNovember 28, 2025 10:17
Sign up for freeto join this conversation on GitHub. Already have an account?Sign in to comment

Reviewers

Copilot code reviewCopilotCopilot left review comments

@tiwarishubham635tiwarishubham635tiwarishubham635 approved these changes

@shrutiburmanshrutiburmanshrutiburman approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@manisha1997@tiwarishubham635@shrutiburman
[8]ページ先頭
©2009-2025 Movatter.jp