Fixed PEP8 issues except E501. #466

blackpioter · 2017-10-27T23:52:27Z

This one is in regard to Issue #464.

SendGridDX · 2017-10-27T23:52:43Z

All committers have signed the CLA.

codecov-io · 2017-10-27T23:59:02Z

Codecov Report

Merging #466 into master will decrease coverage by 0.18%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master     #466      +/-   ##
==========================================
- Coverage    88.1%   87.91%   -0.19%     
==========================================
  Files          30       30              
  Lines         933      935       +2     
  Branches      114      115       +1     
==========================================
  Hits          822      822              
- Misses         54       55       +1     
- Partials       57       58       +1

Impacted Files	Coverage Δ
sendgrid/helpers/inbound/config.py	`93.93% <ø> (ø)`	⬆️
sendgrid/helpers/inbound/send.py	`93.75% <ø> (ø)`	⬆️
sendgrid/helpers/inbound/parse.py	`37.68% <ø> (ø)`	⬆️
sendgrid/helpers/mail/mail.py	`94.77% <0%> (-1.26%)`	⬇️

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 0007d66...29717f3. Read the comment docs.

mbernier

I marked a couple examples of this, can you please move the lists of params to each have their own line? That cleans up the code for readability and it's so nice!

mbernier · 2017-10-28T04:03:52Z

examples/clients/clients.py

@@ -9,7 +9,8 @@
 # Retrieve email statistics by client type. #
 # GET /clients/stats #

-params = {'aggregated_by': 'day', 'start_date': '2016-01-01', 'end_date': '2016-04-01'}
+params = {'aggregated_by': 'day',


Would it be ok to be pedantic here and ask that end_date be moved to the next line too?

mbernier · 2017-10-28T04:03:59Z

examples/clients/clients.py

@@ -19,10 +20,10 @@
 # Retrieve stats by a specific client type. #
 # GET /clients/{client_type}/stats #

-params = {'aggregated_by': 'day', 'start_date': '2016-01-01', 'end_date': '2016-04-01'}
+params = {'aggregated_by': 'day',


Would it be ok to be pedantic here and ask that end_date be moved to the next line too?

mbernier · 2017-10-28T04:04:24Z

examples/contactdb/contactdb.py

 }
 params = {'list_id': 1}
 list_id = "test_url_param"
-response = sg.client.contactdb.lists._(list_id).patch(request_body=data, query_params=params)
+response = sg.client.contactdb.lists._(list_id).patch(
+    request_body=data, query_params=params)


Can you please move query_params to the next line?

mbernier · 2017-10-28T04:04:48Z

examples/contactdb/contactdb.py

 }
 params = {'segment_id': 'test_string'}
 segment_id = "test_url_param"
-response = sg.client.contactdb.segments._(segment_id).patch(request_body=data, query_params=params)
+response = sg.client.contactdb.segments._(segment_id).patch(
+    request_body=data, query_params=params)


Can you please move query_params to the next line?

mbernier · 2017-10-28T04:05:12Z

examples/devices/devices.py

@@ -9,9 +9,9 @@
 # Retrieve email statistics by device type. #
 # GET /devices/stats #

-params = {'aggregated_by': 'day', 'limit': 1, 'start_date': '2016-01-01', 'end_date': '2016-04-01', 'offset': 1}
+params = {'aggregated_by': 'day', 'limit': 1,
+          'start_date': '2016-01-01', 'end_date': '2016-04-01', 'offset': 1}


Can you please move end_date and offset to their own lines?

mbernier · 2017-10-28T04:05:36Z

examples/geo/geo.py

@@ -9,9 +9,9 @@
 # Retrieve email statistics by country and state/province. #
 # GET /geo/stats #

-params = {'end_date': '2016-04-01', 'country': 'US', 'aggregated_by': 'day', 'limit': 1, 'offset': 1, 'start_date': '2016-01-01'}
+params = {'end_date': '2016-04-01', 'country': 'US', 'aggregated_by': 'day',
+          'limit': 1, 'offset': 1, 'start_date': '2016-01-01'}


can you please move the params to each be on their own line?

mbernier · 2017-10-28T04:05:52Z

examples/helpers/mail/mail_example.py

@@ -104,22 +108,28 @@ def build_kitchen_sink():
    mail_settings = MailSettings()
    mail_settings.bcc_settings = BCCSettings(True, Email("[email protected]"))
    mail_settings.bypass_list_management = BypassListManagement(True)
-    mail_settings.footer_settings = FooterSettings(True, "Footer Text", "<html><body>Footer Text</body></html>")
+    mail_settings.footer_settings = FooterSettings(


can you please move the params to each be on their own line?

blackpioter · 2017-10-28T08:09:51Z

@mbernier I've updated PR according to comments. Let me know if you got some more ideas.

mbernier · 2017-10-30T03:31:06Z

Nice! Thank you so much!

mbernier · 2017-10-30T03:47:01Z

Had some merge conflicts. I believe that I resolved them properly. I am letting the build run to verify.

blackpioter · 2017-10-30T17:37:25Z

@mbernier cool. Build seems to be clean after conflicts resolved.

thinkingserious · 2017-11-02T15:53:55Z

@blackpioter

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 · 2018-05-30T17:38:27Z

Hello @blackpioter,

Thanks again for the PR!

We appreciate your contribution and look forward to continued collaboration. Thanks!

Team SendGrid DX

Fixed PEP8 issues except E501.

744e290

thinkingserious added the status: code review request requesting a community code review or review from Twilio label Oct 27, 2017

mbernier suggested changes Oct 28, 2017

View reviewed changes

mbernier added difficulty: hard fix is hard in difficulty hacktoberfest labels Oct 28, 2017

blackpioter and others added 5 commits October 28, 2017 09:50

Updated according to PR comments.

7c19372

Merge branch 'master' into master

853c829

Merge branch 'master' into master

e3ec510

Fix PEP8 E501 where possible.

47de889

Fix PEP8 E501 where possible.

0bd8dc1

Matt Bernier added 2 commits October 29, 2017 21:45

Merge branch 'master' into master

e6af3a3

Merge branch 'master' into master

78a3a05

mbernier approved these changes Oct 30, 2017

View reviewed changes

apigirl added hacktoberfest and removed hacktoberfest labels Oct 31, 2017

thinkingserious added difficulty: easy fix is easy in difficulty type: twilio enhancement feature request on Twilio's roadmap and removed difficulty: hard fix is hard in difficulty labels Feb 27, 2018

Merge branch 'master' into master

29717f3

thinkingserious merged commit 52f86b7 into sendgrid:master May 30, 2018

childish-sambino mentioned this pull request Jul 29, 2020

Pep8 Compliance #464

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixed PEP8 issues except E501. #466

Fixed PEP8 issues except E501. #466

blackpioter commented Oct 27, 2017

SendGridDX commented Oct 27, 2017 •

edited

Loading

codecov-io commented Oct 27, 2017 •

edited by codecov bot

Loading

mbernier left a comment

mbernier Oct 28, 2017

blackpioter Oct 28, 2017

mbernier Oct 28, 2017

blackpioter Oct 28, 2017

mbernier Oct 28, 2017

mbernier Oct 28, 2017

mbernier Oct 28, 2017

mbernier Oct 28, 2017

mbernier Oct 28, 2017

blackpioter Oct 28, 2017

blackpioter commented Oct 28, 2017

mbernier commented Oct 30, 2017

mbernier commented Oct 30, 2017

blackpioter commented Oct 30, 2017

thinkingserious commented Nov 2, 2017

thinkingserious commented May 30, 2018

Fixed PEP8 issues except E501. #466

Fixed PEP8 issues except E501. #466

Conversation

blackpioter commented Oct 27, 2017

SendGridDX commented Oct 27, 2017 • edited Loading

codecov-io commented Oct 27, 2017 • edited by codecov bot Loading

Codecov Report

mbernier left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

blackpioter commented Oct 28, 2017

mbernier commented Oct 30, 2017

mbernier commented Oct 30, 2017

blackpioter commented Oct 30, 2017

thinkingserious commented Nov 2, 2017

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

thinkingserious commented May 30, 2018

SendGridDX commented Oct 27, 2017 •

edited

Loading

codecov-io commented Oct 27, 2017 •

edited by codecov bot

Loading