Skip to content

feat: add missing PhoneNumberToCarrierMapper feature that is available in java version#317

Open
wmundev wants to merge 6 commits intotwcclegg:mainfrom
wmundev:feature/carrier-mapper-phone-number
Open

feat: add missing PhoneNumberToCarrierMapper feature that is available in java version#317
wmundev wants to merge 6 commits intotwcclegg:mainfrom
wmundev:feature/carrier-mapper-phone-number

Conversation

@wmundev
Copy link
Copy Markdown
Collaborator

@wmundev wmundev commented May 2, 2026

Changes

  • feat: add missing PhoneNumberToCarrierMapper feature that is available in java version

@wmundev wmundev self-assigned this May 2, 2026
@wmundev wmundev requested a review from Copilot May 2, 2026 14:21
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

Adds a C# implementation of carrier-name lookup to bring the library closer to libphonenumber’s Java feature set, reusing the existing prefix-mapping/resource-loading model already used for offline geocoding.

Changes:

  • Add PhoneNumberToCarrierMapper and shared PrefixFileReader support for carrier prefix data from embedded folders or zip resources.
  • Expose PhoneNumberUtil.IsMobileNumberPortableRegion and add unit tests covering production, synthetic, and zipped carrier datasets.
  • Wire carrier resources into project files, CI/build scripts, ignore rules, and README usage/testing documentation.

Reviewed changes

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

Show a summary per file
File Description
csharp/PhoneNumbers/PrefixFileReader.cs New shared loader for prefix-based mapping data used by carrier lookups.
csharp/PhoneNumbers/PhoneNumbers.csproj Embeds carrier resources similarly to geocoding resources.
csharp/PhoneNumbers/PhoneNumberUtil.cs Adds region-level mobile-number-portability lookup API.
csharp/PhoneNumbers/PhoneNumberToCarrierMapper.cs Introduces the public carrier-mapper API and singleton access.
csharp/PhoneNumbers.Test/TestPhoneNumberToCarrierMapper.cs Adds production/test/zipped-path coverage for carrier lookup behavior.
csharp/PhoneNumbers.Test/PhoneNumbers.Test.csproj Embeds test carrier resources and zipped test archive.
appveyor.yml Updates Windows CI packaging steps for carrier resources.
README.md Documents carrier lookup usage and local test zip generation.
.gitignore Ignores generated carrier zip artifacts.
.github/workflows/run_all_tests_and_upload_code_coverage.yml Generates zipped test carrier data for Windows GitHub Actions.
.github/workflows/build_and_run_unit_tests_linux.yml Generates zipped test carrier data for Linux GitHub Actions.

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

Comment thread appveyor.yml Outdated
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

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


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

Comment thread README.md
Comment thread csharp/PhoneNumbers.Test/PhoneNumbers.Test.csproj Outdated
@wmundev wmundev requested a review from twcclegg May 2, 2026 15:06
@wmundev wmundev marked this pull request as ready for review May 2, 2026 15:06
@wmundev wmundev force-pushed the feature/carrier-mapper-phone-number branch from 46876b4 to aec7dd5 Compare May 3, 2026 12:58
@codecov
Copy link
Copy Markdown

codecov Bot commented May 3, 2026

Codecov Report

❌ Patch coverage is 97.33333% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.79%. Comparing base (aa1e91e) to head (9dcec9b).

Files with missing lines Patch % Lines
csharp/PhoneNumbers/PrefixFileReader.cs 96.22% 0 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #317      +/-   ##
==========================================
+ Coverage   75.31%   75.79%   +0.47%     
==========================================
  Files          38       40       +2     
  Lines        4638     4713      +75     
  Branches     1097     1114      +17     
==========================================
+ Hits         3493     3572      +79     
+ Misses        917      912       -5     
- Partials      228      229       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread csharp/PhoneNumbers/PrefixFileReader.cs Outdated
Comment thread resources/test/carrier/en/1650.txt
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

Copilot reviewed 10 out of 11 changed files in this pull request and generated 3 comments.


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

Comment thread .github/workflows/build_and_run_unit_tests_linux.yml Outdated
Comment thread .github/workflows/run_all_tests_and_upload_code_coverage.yml Outdated
Comment thread csharp/PhoneNumbers/PhoneNumberUtil.cs
@wmundev
Copy link
Copy Markdown
Collaborator Author

wmundev commented May 4, 2026

i'll hold off on merging this until #323 is merged

wmundev added 3 commits May 5, 2026 22:34
…per-phone-number

# Conflicts:
#	.github/workflows/build_and_run_unit_tests_linux.yml
#	.github/workflows/run_all_tests_and_upload_code_coverage.yml
#	appveyor.yml
#	csharp/PhoneNumbers.Test/PhoneNumbers.Test.csproj
#	csharp/PhoneNumbers/PhoneNumbers.csproj
"test" => BuildPerRegion(input, output, TestMetadataPrefix,
isShortNumberMetadata: false, isAlternateFormatsMetadata: false),
"geocoding" => BuildGeocoding(input, output),
"carrier" => BuildGeocoding(input, output),
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

BuildPrefixMap?

/// <returns>A carrier name for the given phone number, or empty string if not found.</returns>
public string GetNameForValidNumber(PhoneNumber number, Locale languageCode)
{
return prefixFileReader.GetDescriptionForNumber(
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Add:

      // No script is specified — Java's Locale.getScript() has no equivalent on this
      // port's Locale type. Matches the same omission in PhoneNumberOfflineGeocoder. 

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.

3 participants