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

Add attrs export to python client #262

Merged
merged 12 commits into from Jan 18, 2022
Merged

Conversation

sderickson
Copy link
Contributor

@sderickson sderickson commented Jan 8, 2022

Update the python client generator, adding a command line arg to export route attributes to the documentation string. This way generated docs can include specified attributes, in our case the scope attribute.

Checklist

General Contributing

  • Have you read the Code of Conduct and signed the CLA?

Is This a Code Change?

  • Non-code related change (markdown/git settings etc)
  • Code Change
  • Example/Test Code Change

Validation

  • Have you ran tox?
  • Do the tests pass?

@codecov
Copy link

codecov bot commented Jan 8, 2022

Codecov Report

Merging #262 (33252e0) into main (b7b6432) will increase coverage by 0.24%.
The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #262      +/-   ##
==========================================
+ Coverage   51.65%   51.90%   +0.24%     
==========================================
  Files          37       37              
  Lines        8425     8437      +12     
  Branches     1794     1800       +6     
==========================================
+ Hits         4352     4379      +27     
+ Misses       3757     3741      -16     
- Partials      316      317       +1     
Flag Coverage Δ
unit 51.90% <93.33%> (+0.24%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
stone/backends/python_client.py 46.66% <93.33%> (+8.29%) ⬆️

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 b7b6432...33252e0. Read the comment docs.

Copy link
Contributor

@rogebrd rogebrd left a comment

Choose a reason for hiding this comment

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

Overall methodology looks sound, just a few clarifying things

stone/backends/python_client.py Outdated Show resolved Hide resolved
test/test_python_client.py Show resolved Hide resolved
@aelawson
Copy link
Contributor

Seems reasonable as a PoC. Let me know once you've got it in a state ready for a full review.

Scott Erickson and others added 3 commits January 12, 2022 13:58
* attribute -> attribute-comment
* attrs_list -> attrs_lines
* added a test for a line that wasn't covered
* made sure to test multiple attributes being included
Copy link
Contributor

@aelawson aelawson left a comment

Choose a reason for hiding this comment

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

LGTM

@sderickson sderickson merged commit 45770eb into main Jan 18, 2022
@sderickson sderickson deleted the add-attrs-export-to-python-client branch January 18, 2022 18:00
sderickson added a commit that referenced this pull request Jan 25, 2022
In testing 3.3.0 with the Python SDK, turns out routes without the attribute defined have values of "None". This PR tweaks the logic from #262 to handle None, and adds a regression test.

Also updates the version to so as a follow I can publish the fix.
sderickson added a commit that referenced this pull request Feb 1, 2022
Add --attribute-comment field to js_client.py and tsd_client.py, like the one added to python_client.py in #262. This is a prerequisite for dropbox/dropbox-sdk-js#891 which adds scopes to the outputted clients for easier reference.

Also adds appropriate testing and updates the docstring for the python_client.py, making it more succinct and aligned with the other two.
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.

None yet

3 participants