Skip to content
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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allows users to submit rfc822 formatted email addresses #348

Merged
merged 10 commits into from
Oct 23, 2017

Conversation

mbernier
Copy link
Contributor

@mbernier mbernier commented Oct 1, 2017

Closes #277

@SendGridDX
Copy link

SendGridDX commented Oct 1, 2017

CLA assistant check
All committers have signed the CLA.

@thinkingserious thinkingserious added status: code review request requesting a community code review or review from Twilio difficulty: medium fix is medium in difficulty hacktoberfest labels Oct 1, 2017
@mbernier
Copy link
Contributor Author

mbernier commented Oct 2, 2017

@thinkingserious I am not sure what happened with the Travis build... looks like there's an env problem?

self.email = email
if name is not None:
self.name = name
if not name
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be if not name:

@mbernier
Copy link
Contributor Author

mbernier commented Oct 2, 2017

Updated, thanks!

Also realized, we already checked that name exists... so we don't have to check again
The class was not using get/set methods for all calls. As well, email and name were not initialized. This is now rectified.
@codecov-io
Copy link

codecov-io commented Oct 22, 2017

Codecov Report

Merging #348 into master will increase coverage by 0.43%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #348      +/-   ##
==========================================
+ Coverage   81.54%   81.98%   +0.43%     
==========================================
  Files           9        9              
  Lines         981      999      +18     
  Branches      156      160       +4     
==========================================
+ Hits          800      819      +19     
  Misses         90       90              
+ Partials       91       90       -1
Impacted Files Coverage Δ
sendgrid/helpers/mail/mail.py 88.27% <100%> (+0.39%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0884519...3f876af. Read the comment docs.

email class now allows empty instantiation without error
If a name is passed, i.e. an email with no '@', then the name gets set rather than defaulting to email param
@mbernier
Copy link
Contributor Author

mbernier commented Oct 23, 2017

@thinkingserious I added a ridiculous number of tests and even found something I missed. You win. Fine. Are you happy!!? 🥇

@thinkingserious thinkingserious merged commit ae54113 into master Oct 23, 2017
@thinkingserious
Copy link
Contributor

Hello @mbernier,

Thanks again for the PR!

It's HACKTOBERFEST! We want to show our appreciation by sending you some special Hacktoberfest swag. If you have not already, could you please fill out this form so we can send it to you? Thanks!

Team SendGrid DX

@mbernier
Copy link
Contributor Author

It's MERGED!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
difficulty: medium fix is medium in difficulty status: code review request requesting a community code review or review from Twilio
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants