Remove Yahoo dash address normalization#14
Conversation
Yahoo dash addresses (e.g. nickname-keyword@) are distinct accounts, not aliases of the base address (nickname@). The DASH_ADDRESSING rule was incorrectly normalizing them as equivalent, causing unrelated addresses to be treated as the same. Base addresses themselves are not deliverable inboxes. This change removes all normalization rules from the Yahoo provider. The provider class remains to explicitly denote that Yahoo addresses are not to be normalized.
📝 WalkthroughWalkthroughThe PR removes dash-based local-part normalization by deleting DASH_ADDRESSING usage and associated splitting logic, replaces the Rules enum member with NONE, updates the Yahoo provider to use Rules.NONE, adjusts the Yahoo test expectation, and adds a pull_request trigger to CI workflow. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/test_normalize.py (1)
108-115: Test update is correct.
uuid.uuid4()always produces a dash-containing string (e.g.,a1b2c3d4-e5f6-...), so this test implicitly verifies that a dash-containinglocal_partis no longer truncated at the first dash. The updated assertion correctly reflects the new no-normalization behavior.As a minor readability improvement, an explicit
nickname-keyword@stylelocal_partwould make the intent of the test self-evident without relying on knowledge of UUID formatting:✨ Optional: explicit dash-address test
def test_yahoo(self): - local_part = str(uuid.uuid4()) - domain_part = str(uuid.uuid4()) - address = '{}@{}'.format(local_part, domain_part) + # Dash addresses (nickname-keyword@) are distinct accounts, not aliases + nickname = str(uuid.uuid4()) + local_part = '{}-keyword'.format(nickname) + domain_part = str(uuid.uuid4()) + address = '{}@{}'.format(local_part, domain_part) mx_records = [(1, 'mta5.am0.yahoodns.net')] self._perform_test( address, '{}@{}'.format(local_part, domain_part), mx_records, 'Yahoo')🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/test_normalize.py` around lines 108 - 115, Replace the generated uuid local_part in test_yahoo with a clear dash-containing literal (e.g., "nickname-keyword") so the intent is explicit; update the local_part and address variables used in the call to self._perform_test(address, '{}@{}'.format(local_part, domain_part), mx_records, 'Yahoo') while keeping mx_records and the expected result unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/test_normalize.py`:
- Around line 108-115: Replace the generated uuid local_part in test_yahoo with
a clear dash-containing literal (e.g., "nickname-keyword") so the intent is
explicit; update the local_part and address variables used in the call to
self._perform_test(address, '{}@{}'.format(local_part, domain_part), mx_records,
'Yahoo') while keeping mx_records and the expected result unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
email_normalize/__init__.pyemail_normalize/providers.pytests/test_normalize.py
💤 Files with no reviewable changes (1)
- email_normalize/init.py
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/testing.yaml (1)
49-53:⚠️ Potential issue | 🟠 MajorUpgrade
codecov/codecov-action@v1.0.2— this version was sunset on February 1, 2022 and no longer functions.Coverage uploads will fail on every push to any branch. Since
fail_ci_if_error: trueis enabled (line 54), these failures will block CI.Upgrade to the current version
@v5(or@v5.5.2for exact pinning):Proposed fix
- uses: codecov/codecov-action@v1.0.2 + uses: codecov/codecov-action@v5🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/testing.yaml around lines 49 - 53, Replace the deprecated Codecov action reference used in the workflow (uses: codecov/codecov-action@v1.0.2) with the supported v5 release (e.g., uses: codecov/codecov-action@v5 or `@v5.5.2`) so coverage uploads no longer fail; keep the existing inputs (token: ${{secrets.CODECOV_TOKEN}} and file: build/coverage.xml) and any related flags (like fail_ci_if_error: true) unchanged unless the new action docs indicate a renamed input, in which case update the input name to match the v5 parameter names.
🧹 Nitpick comments (1)
.github/workflows/testing.yaml (1)
28-28:actions/checkout@v1is deprecated — upgrade to@v4.These actions will start to fail after January 30, 2025. While this is a pre-existing issue, since the workflow file is being touched in this PR it's a good opportunity to upgrade.
🔧 Proposed fix
- uses: actions/checkout@v1 + uses: actions/checkout@v4🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/testing.yaml at line 28, The workflow is using the deprecated action reference "uses: actions/checkout@v1"; update that step to use the maintained release "actions/checkout@v4" (replace the string "actions/checkout@v1" with "actions/checkout@v4" in the workflow) and verify any inputs remain compatible with Checkout v4 in the same job step where "uses: actions/checkout@v1" appears.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/testing.yaml:
- Around line 49-53: Replace the deprecated Codecov action reference used in the
workflow (uses: codecov/codecov-action@v1.0.2) with the supported v5 release
(e.g., uses: codecov/codecov-action@v5 or `@v5.5.2`) so coverage uploads no longer
fail; keep the existing inputs (token: ${{secrets.CODECOV_TOKEN}} and file:
build/coverage.xml) and any related flags (like fail_ci_if_error: true)
unchanged unless the new action docs indicate a renamed input, in which case
update the input name to match the v5 parameter names.
---
Nitpick comments:
In @.github/workflows/testing.yaml:
- Line 28: The workflow is using the deprecated action reference "uses:
actions/checkout@v1"; update that step to use the maintained release
"actions/checkout@v4" (replace the string "actions/checkout@v1" with
"actions/checkout@v4" in the workflow) and verify any inputs remain compatible
with Checkout v4 in the same job step where "uses: actions/checkout@v1" appears.
2 participants
Yahoo dash addresses (e.g. nickname-keyword@) are distinct accounts, not aliases of the base address (nickname@). The DASH_ADDRESSING rule was incorrectly normalizing them as equivalent, causing unrelated addresses to be treated as the same. Base addresses themselves are not deliverable inboxes.
This change removes all normalization rules from the Yahoo provider. The provider class remains to continue identifying Yahoo addresses and to explicitly denote that they are not to be normalized.
Some info on this https://help.yahoo.com/kb/SLN28815.html
Summary by CodeRabbit