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

refactor personalization.py as in issue #501 #493

Closed
wants to merge 8 commits into from

Conversation

defaults
Copy link

@defaults defaults commented Oct 29, 2017

reduce Cognitive Complexity of get method and refactor init and other methods of personalization class

Fixes #501

Checklist

  • I have made a material change to the repo (functionality, testing, spelling, grammar)
  • I have read the [Contribution Guide] and my PR follows them.
  • I updated my branch with the master branch.
  • I have added tests that prove my fix is effective or that my feature works
  • I have added necessary documentation about the functionality in the appropriate .md file
  • I have added inline documentation to the code I modified

Short description of what this PR does:

  • Reduce Cognitive Complexity of get method
  • Refactor to remove similar code block at many places

If you have questions, please send an email to Sendgrid, or file a Github Issue in this repository.

reduce Cognitive Complexity of get method and refactor init and other methods of personalization class
@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 29, 2017
@defaults defaults changed the title refactor personalization.py issue #487 refactor personalization.py as in issue #487 Oct 29, 2017
@codecov-io
Copy link

codecov-io commented Oct 29, 2017

Codecov Report

Merging #493 into master will decrease coverage by 0.07%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #493      +/-   ##
==========================================
- Coverage   87.91%   87.83%   -0.08%     
==========================================
  Files          30       30              
  Lines         935      929       -6     
  Branches      115      113       -2     
==========================================
- Hits          822      816       -6     
  Misses         55       55              
  Partials       58       58
Impacted Files Coverage Δ
sendgrid/helpers/mail/personalization.py 90.78% <100%> (-0.68%) ⬇️

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 52f86b7...191e109. Read the comment docs.

@thinkingserious thinkingserious added difficulty: medium fix is medium in difficulty hacktoberfest labels Oct 29, 2017
@defaults defaults changed the title refactor personalization.py as in issue #487 refactor personalization.py as in issue #501 Oct 30, 2017
@defaults
Copy link
Author

@thinkingserious @mbernier hey, this PR is open for around a day. Can we do code review and merge the changes. Being my first open source contribution to a popular repo used by many, I am a bit excited to see it merged.

@defaults
Copy link
Author

defaults commented Oct 30, 2017

@thinkingserious @mbernier do you guys really want people to contribute to your repo? I have this Pr for two days and have pinged multiple times on issue and PR to code review or merge and I see no reply but other PRs are being merged and here I am back merging and resolving conflicts. Don't you guys have the courtesy to reply at least?

People like you and these shitty repos demotivate open source contributors. Don't be an ass! Do hell with your hacktoberfest.

@defaults
Copy link
Author

@thinkingserious please at least close the repo if not needed!

@mbernier
Copy link
Contributor

mbernier commented Nov 2, 2017

@Codervikash

We have not been able to merge your Pull Request, but because you are awesome - we wanted to make sure you could still get a SendGrid Hacktoberfest shirt.

Please go fill out our swag form before Nov 5th and we will send the shirt! (We know that you might have tried this before and it didn’t work, sorry about that!)

You have till Nov 5th to fill out this form in order to get the Hacktoberfest shirt!

Thank you for contributing during Hacktoberfest! We hope to see you in the repos soon! Just so you know, we always give away a SendGrid shirt for your first ever non-Hacktoberfest PR that gets merged.

@defaults
Copy link
Author

defaults commented Nov 2, 2017

@mbernier Hey, thanks and sorry for being rude! I was overexcited to see my first contribution to a well known and used repo merged. Leaving aside t-shirt, I would like to get this merged. Please do a code review whenever free and suggest changes. Travis is falling after back-merge, due to some issue in master. I will try fixing that also. Thanks for the response.

@mbernier
Copy link
Contributor

mbernier commented Nov 6, 2017

No worries @Codervikash! We appreciate your excitement and hope that you keep an eye on our repos going forward!

@thinkingserious thinkingserious added the type: twilio enhancement feature request on Twilio's roadmap label Feb 27, 2018
thinkingserious added a commit that referenced this pull request Jul 12, 2018
@thinkingserious
Copy link
Contributor

Thanks again for helping improve this SDK!

We have started to execute on PR #347 (Python Mail Helper Refactor) and have integrated your work into this branch. Please follow along at PR #347 to follow our progress.

With Best Regards,

Elmer

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 type: twilio enhancement feature request on Twilio's roadmap
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix "similar-code" issue in sendgrid/helpers/mail/personalization.py
5 participants