Skip to content

Email validation api#26

Merged
fpseverino merged 24 commits into
vapor-community:mainfrom
thoven87:email-validation-api
May 17, 2025
Merged

Email validation api#26
fpseverino merged 24 commits into
vapor-community:mainfrom
thoven87:email-validation-api

Conversation

@thoven87
Copy link
Copy Markdown
Contributor

@codecov
Copy link
Copy Markdown

codecov Bot commented May 14, 2025

Codecov Report

Attention: Patch coverage is 77.23577% with 28 lines in your changes missing coverage. Please review.

Project coverage is 89.47%. Comparing base (078282f) to head (ee49673).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...es/SendGridKit/SendGridEmailValidationClient.swift 75.65% 28 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (078282f) and HEAD (ee49673). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (078282f) HEAD (ee49673)
3 2
Additional details and impacted files
@@             Coverage Diff              @@
##              main      #26       +/-   ##
============================================
- Coverage   100.00%   89.47%   -10.53%     
============================================
  Files            9       12        +3     
  Lines          145      266      +121     
============================================
+ Hits           145      238       +93     
- Misses           0       28       +28     
Files with missing lines Coverage Δ
...urces/SendGridKit/Models/BulkEmailValidation.swift 100.00% <100.00%> (ø)
...s/SendGridKit/Models/RealtimeEmailValidation.swift 100.00% <100.00%> (ø)
Sources/SendGridKit/SendGridClient.swift 100.00% <100.00%> (ø)
...es/SendGridKit/SendGridEmailValidationClient.swift 75.65% <75.65%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@thoven87
Copy link
Copy Markdown
Contributor Author

Phew -40% in code coverage dropped? Let's see if I can fix that :(

Copy link
Copy Markdown
Member

@fpseverino fpseverino left a comment

Choose a reason for hiding this comment

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

Great job, I am very excited to add this API to the library!
I left some comments, thank you very much for the PR!

Comment thread Sources/SendGridKit/Models/EmailValidation.swift Outdated
Comment thread Sources/SendGridKit/Models/EmailValidation.swift Outdated
Comment thread Sources/SendGridKit/Models/EmailValidation.swift Outdated
Comment thread Sources/SendGridKit/Models/EmailValidation.swift Outdated
Comment thread Sources/SendGridKit/Models/EmailValidation.swift Outdated
Comment thread Tests/SendGridKitTests/SendGridKitTests.swift Outdated
Comment thread Sources/SendGridKit/Models/EmailValidation.swift Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
Comment thread README.md Outdated
@thoven87
Copy link
Copy Markdown
Contributor Author

Great job, I am very excited to add this API to the library! I left some comments, thank you very much for the PR!

Thanks @fpseverino I will address the rest comments in a few.

thoven87 and others added 5 commits May 14, 2025 21:10
@thoven87 thoven87 requested a review from fpseverino May 15, 2025 01:30
thoven87 and others added 8 commits May 14, 2025 21:37
Co-authored-by: Francesco Paolo Severino <96546612+fpseverino@users.noreply.github.com>
Co-authored-by: Francesco Paolo Severino <96546612+fpseverino@users.noreply.github.com>
Co-authored-by: Francesco Paolo Severino <96546612+fpseverino@users.noreply.github.com>
Co-authored-by: Francesco Paolo Severino <96546612+fpseverino@users.noreply.github.com>
Co-authored-by: Francesco Paolo Severino <96546612+fpseverino@users.noreply.github.com>
@thoven87
Copy link
Copy Markdown
Contributor Author

thoven87 commented May 15, 2025

Great job, I am very excited to add this API to the library! I left some comments, thank you very much for the PR!

Scratch what I said previously. How do we update uploadBulkValidationFile for better code coverage? Or are you ok with the current drop in code coverage?

Copy link
Copy Markdown
Member

@fpseverino fpseverino left a comment

Choose a reason for hiding this comment

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

A few more comments. Also, for all response types we have to make all properties optional, as we can't assume they will all be present when decoding them.

Comment thread Sources/SendGridKit/Models/RealtimeEmailValidation.swift Outdated
Comment thread Sources/SendGridKit/Models/RealtimeEmailValidation.swift Outdated
Comment thread Sources/SendGridKit/Models/BulkEmailValidation.swift Outdated
Comment thread Sources/SendGridKit/Models/BulkEmailValidation.swift Outdated
Comment thread Sources/SendGridKit/Models/BulkEmailValidation.swift Outdated
Comment thread Sources/SendGridKit/Models/BulkEmailValidation.swift Outdated
Comment thread Sources/SendGridKit/Models/BulkEmailValidation.swift Outdated
Comment thread Sources/SendGridKit/Models/BulkEmailValidation.swift Outdated
Copy link
Copy Markdown
Member

@fpseverino fpseverino left a comment

Choose a reason for hiding this comment

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

How do we update uploadBulkValidationFile for better code coverage? Or are you ok with the current drop in code coverage?

For now, it is not important. The more you can raise it the better

Copy link
Copy Markdown
Member

@fpseverino fpseverino left a comment

Choose a reason for hiding this comment

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

Looking much better, we're almost there! Thanks again

Comment thread Tests/SendGridKitTests/SendGridKitTests.swift Outdated
Comment thread Sources/SendGridKit/SendGridEmailValidationClient.swift
Comment thread Sources/SendGridKit/SendGridEmailValidationClient.swift Outdated
@thoven87 thoven87 requested a review from fpseverino May 16, 2025 15:38
Copy link
Copy Markdown
Member

@fpseverino fpseverino left a comment

Choose a reason for hiding this comment

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

OK, let's merge it!

A PR of mine will follow, before the official release, in which I will update the DocC documentation and other such things.
I warn you that I may change the names of some methods or structures.

Thank you once again!

@thoven87
Copy link
Copy Markdown
Contributor Author

OK, let's merge it!

A PR of mine will follow, before the official release, in which I will update the DocC documentation and other such things.

I warn you that I may change the names of some methods or structures.

Thank you once again!

Unfortunately, you'll need to merge. I don't have permissions todo so.

@fpseverino fpseverino merged commit 76d442f into vapor-community:main May 17, 2025
13 of 14 checks passed
@thoven87 thoven87 deleted the email-validation-api branch September 12, 2025 13:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants