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

[FIX] Send data via JSON in call #8

Merged
merged 1 commit into from Dec 27, 2016
Merged

Conversation

lasley
Copy link
Member

@lasley lasley commented Dec 24, 2016

No description provided.

@codecov-io
Copy link

codecov-io commented Dec 25, 2016

Current coverage is 100% (diff: 100%)

Merging #8 into master will not change coverage

@@           master    #8   diff @@
===================================
  Files           4     4          
  Lines          85    86     +1   
  Methods         0     0          
  Messages        0     0          
  Branches        3     3          
===================================
+ Hits           85    86     +1   
  Misses          0     0          
  Partials        0     0          

Sunburst

Diff Coverage File Path
•••••••••• 100% red_october/red_october.py

Powered by Codecov. Last update d3498cf...8b94611

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 390dda4b32c5a0a36d49876af807e4989677cd1b on bugfix/master/response-parse into d53fe77 on master.

@lasley lasley force-pushed the bugfix/master/response-parse branch from 390dda4 to 34107fb Compare December 27, 2016 20:56
@lasley lasley force-pushed the bugfix/master/response-parse branch from 34107fb to 8b94611 Compare December 27, 2016 21:00
@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 8b94611 on bugfix/master/response-parse into d3498cf on master.

@lasley
Copy link
Member Author

lasley commented Dec 27, 2016

Rebased to fix conflicts; ready for review

Copy link
Contributor

@tedsalmon tedsalmon left a comment

Choose a reason for hiding this comment

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

Minor suggestion. Otherwise, LGTM! 👍

'Name': self.name,
'Password': self.password,
})
if data is None:
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not save two lines by setting the default of data to {} in the method declaration?

Copy link
Member Author

Choose a reason for hiding this comment

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

We have a lint check to prevent this, but here is a little lesson in mutability 😉 :

>>> def test(key, val, map={}):
...     map[key] = val
...     print map
... 
>>> test('a', 'a-val')
{'a': 'a-val'}
>>> test('b', 'b-val')
{'a': 'a-val', 'b': 'b-val'}

Copy link
Contributor

@tedsalmon tedsalmon Dec 27, 2016

Choose a reason for hiding this comment

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

Derppppp! I totally forgot about that. The worst part is, I think this bit one of us in the butt with a project way back when.

@lasley lasley merged commit 4b28402 into master Dec 27, 2016
@lasley lasley deleted the bugfix/master/response-parse branch December 27, 2016 22:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants