Skip to content

Conversation

petrklus
Copy link

Done and tested:

  • Updated function signatures to support SendGrid v6
  • Updated tests to match what was expected (omission of empty subject from Personalization)
  • Tested sending with Django (tested adding attachments

What has not been tested:

  • Templates using template_id (as I am not using any)
  • Substitutions (again, not using any)

I think this should now require fairly small amount of work to get fully tested

@codecov-io
Copy link

codecov-io commented Oct 27, 2020

Codecov Report

Merging #95 into master will increase coverage by 0.62%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #95      +/-   ##
==========================================
+ Coverage   77.35%   77.98%   +0.62%     
==========================================
  Files           1        1              
  Lines         106      109       +3     
  Branches       30       30              
==========================================
+ Hits           82       85       +3     
  Misses         19       19              
  Partials        5        5              
Impacted Files Coverage Δ
sgbackend/mail.py 77.98% <66.66%> (+0.62%) ⬆️

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 aeb46bd...af893d5. Read the comment docs.

@andriisoldatenko
Copy link
Collaborator

@petrklus please update codecov.

@andriisoldatenko andriisoldatenko self-requested a review October 27, 2020 21:20
@petrklus
Copy link
Author

thank you for the quick reply @andriisoldatenko - do you mean that I should write brand new tests for the file attachment dynamic, as well as substitutions?
Considering that the code was untested on master before, is that really necessary to implement new tests as a requirement for the PR to be merged? Curious!

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