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

Custom_Args Feature #53

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Custom_Args Feature #53

wants to merge 6 commits into from

Conversation

lucassimon
Copy link

Added Custom_Args Feature to get the values in webhook.

How can I add unit test and run?

I updated the version of sendgrid and because of this I had to change
how data is assigned to the helpers (Mail, Personalization and
Attachment) classes.

I also added the CustomArgs helper to be sent to sendgrid.
Added custom_args feature
@codecov-io
Copy link

codecov-io commented May 4, 2017

Codecov Report

Merging #53 into master will increase coverage by 1.72%.
The diff coverage is 57.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
+ Coverage   60.22%   61.95%   +1.72%     
==========================================
  Files           1        1              
  Lines          88       92       +4     
  Branches       22       25       +3     
==========================================
+ Hits           53       57       +4     
  Misses         29       29              
  Partials        6        6
Impacted Files Coverage Δ
sgbackend/mail.py 61.95% <57.14%> (+1.72%) ⬆️

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 476bfbe...8df68d5. Read the comment docs.

@@ -14,7 +14,7 @@
license='MIT',
description='SendGrid Backend for Django',
long_description=open('./README.rst').read(),
install_requires=["sendgrid >= 3.5, < 4"],
install_requires=["sendgrid >= 3.5, < 4.0.4"],

Choose a reason for hiding this comment

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

This change implies that django-sendgrid is compatible with sendgrid 4. Is this actually the case? And if so, why don't we correct the entire version range?

Copy link
Author

Choose a reason for hiding this comment

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

Yes the latest version of the python-sendgrid package is version 4.0.0. It was unnecessary that I changed it to 4.0.4. I do not remember why I did it.

$ Pip freeze | Grep -i sendgrid
Sendgrid == 4.0.0
-e git + https: //github.com/lucassimon/sendgrid-django.git@8df68d527f1783392dca7e84bbf04606120c4ac7#egg=sendgrid_django

And if so, why do not we fix the whole range of versions?

What are the other versions that should be changed? The same dependency is only from python-sendgrid and changed the main version of this package:

Choose a reason for hiding this comment

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

Yes, I understand that the python-sendgrid package has hit version 4. I also understand that we previously did not allow django-sendgrid to be installed alongside python-sendgrid version 4.

This change now allows for django-sendgrid to be installed along python-sendgrid version 4. Does django-sendgrid actually support python-sendgrid version 4?

@camflan
Copy link
Contributor

camflan commented Nov 21, 2017

I've opened a new PR, #76, that handles this with tests and passes all checks for the project. Just waiting on @elbuo8

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.

4 participants