Skip to content

Conversation

@Tranquilled
Copy link

@Tranquilled Tranquilled commented Nov 1, 2017

Created skeleton functions from which to create subfunctions; in reality, all of these functions should be replaced. However, changing function names throughout should be its own issue.

Fixes

Closes #46

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 in line documentation to the code I modified

Short description of what this PR does:

  • Creates 2 skeleton functions
  • Replaces the bodies of 2 functions with those skeleton functions to retain namespace

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

…ity, all of these functions should be replaced. However, changing function names throughout should be its own issue.
@thinkingserious thinkingserious added the status: code review request requesting a community code review or review from Twilio label Nov 1, 2017
@SendGridDX
Copy link

SendGridDX commented Nov 1, 2017

CLA assistant check
All committers have signed the CLA.

@mbernier
Copy link
Contributor

mbernier commented Nov 3, 2017

@Tranquilled

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.

@thinkingserious thinkingserious added type: community enhancement feature request not on Twilio's roadmap difficulty: easy fix is easy in difficulty labels Feb 27, 2018
def add_attr(self, key, value, attr):
if attr not in self.data:
self.data[attr] = {}
self.data[attr][key] = value
Copy link

Choose a reason for hiding this comment

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

Maybe use EAFP instead of LBYL?

try:
    self.data[attr][key] = value
except KeyError:
    self.data[attr] = {key: value}

def append_attr(self, value, attr):
if attr not in self.data:
self.data[attr] = []
self.data[attr].append(value)
Copy link

Choose a reason for hiding this comment

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

EAFP over LBYL?

try:
    self.data[attr].append(value)
except KeyError:
    self.data[attr] = [value]

if 'category' not in self.data:
self.data['category'] = []
self.data['category'].append(category)
append_attr(self, category, 'category')
Copy link

Choose a reason for hiding this comment

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

Is there some advantage to calling the method in the above manner instead of like this?

self.append_attr(category, 'category')

if 'section' not in self.data:
self.data['section'] = {}
self.data['section'][key] = section
add_attr(self, key, section, 'section')
Copy link

Choose a reason for hiding this comment

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

self.add_attr(key, section, 'section')  # ?

Copy link

@misterdorm misterdorm left a comment

Choose a reason for hiding this comment

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

@Tranquilled Suggest going with the changes raised by @42B here, for some added cleanliness. Thanks for this work!!

@misterdorm
Copy link

Please also rebase to address conflicts.

@misterdorm misterdorm added status: work in progress Twilio or the community is in the process of implementing and removed status: code review request requesting a community code review or review from Twilio labels Oct 31, 2018
@misterdorm
Copy link

misterdorm commented Oct 31, 2018

Duplicate of #55 and #53

@misterdorm misterdorm marked this as a duplicate of #55 Oct 31, 2018
@misterdorm misterdorm added the status: duplicate duplicate issue label Oct 31, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

difficulty: easy fix is easy in difficulty status: duplicate duplicate issue status: work in progress Twilio or the community is in the process of implementing type: community enhancement feature request not on Twilio's roadmap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix "similar-code" issue in smtpapi/__init__.py

6 participants