Skip to content

Get rid of python2 support#61

Merged
edif2008 merged 1 commit intomainfrom
drop_python2_support
Jan 4, 2023
Merged

Get rid of python2 support#61
edif2008 merged 1 commit intomainfrom
drop_python2_support

Conversation

@volodymyrZotov
Copy link
Copy Markdown
Contributor

Users ask to get rid of python2 support as it is not possible for them to use this Connect client in new projects.

The changes are done in PR:

  • Remove six package.
  • Update all dependencies to the latest versions.

The package is functionally tested. No issues were found after applying the changes.

Resolves #59

Signed-off-by: volodymyrZotov <volodymyr.zotov@gmail.com>
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

Base: 76.29% // Head: 76.11% // Decreases project coverage by -0.17% ⚠️

Coverage data is based on head (3a3ddb6) compared to base (5e70b02).
Patch coverage: 14.28% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #61      +/-   ##
==========================================
- Coverage   76.29%   76.11%   -0.18%     
==========================================
  Files          21       21              
  Lines        1599     1587      -12     
==========================================
- Hits         1220     1208      -12     
  Misses        379      379              
Impacted Files Coverage Δ
src/onepasswordconnectsdk/models/error.py 40.00% <0.00%> (-1.18%) ⬇️
src/onepasswordconnectsdk/models/field.py 82.60% <0.00%> (-0.15%) ⬇️
src/onepasswordconnectsdk/models/field_section.py 53.65% <0.00%> (-1.11%) ⬇️
src/onepasswordconnectsdk/models/file.py 40.90% <0.00%> (-0.89%) ⬇️
src/onepasswordconnectsdk/models/item.py 86.62% <0.00%> (-0.09%) ⬇️
src/onepasswordconnectsdk/models/item_details.py 40.00% <0.00%> (-1.18%) ⬇️
src/onepasswordconnectsdk/models/item_urls.py 40.81% <0.00%> (-1.19%) ⬇️
src/onepasswordconnectsdk/models/item_vault.py 52.50% <0.00%> (-1.16%) ⬇️
src/onepasswordconnectsdk/models/section.py 61.22% <0.00%> (-0.78%) ⬇️
src/onepasswordconnectsdk/models/summary_item.py 78.57% <0.00%> (-0.16%) ⬇️
... and 2 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@edif2008
Copy link
Copy Markdown
Member

edif2008 commented Jan 3, 2023

This looks good to me. A couple of questions before approving this PR:

  • Have you checked whether the current versions of the dependencies support only Python 3.7 or higher? (just to double-check)
  • Are there any dependencies that we no longer need? It may be a good cleanup opportunity with this PR.
  • Any idea why the test coverage dropped? I know it's very small (by 0.17%), but would like to know the reason for it.

Also, one small notice. Since you've removed the six dependency in a couple of files, now we have two new lines between the imports and the code itself. I believe having only one is enough, so you can remove the extra newline as well.

@volodymyrZotov
Copy link
Copy Markdown
Contributor Author

@edif2008 Thank you for reviewing!

  1. It works with higher versions. The latest I tried was v3.10.5.
  2. Nothing to clean up right now. We need all of them.
  3. Good question. Don't have an exact answer, unfortunately. The detailed test report says nothing also.

And regarding the code formatting.
Pycharm suggest to use new 2 lines between imports section and the rest of the code PEP 8: E302 expected 2 blank lines, found 1. I'd stick with 2, as it was before the changes also.

@edif2008 edif2008 merged commit ee95ce1 into main Jan 4, 2023
@edif2008 edif2008 deleted the drop_python2_support branch January 4, 2023 12:29
@volodymyrZotov volodymyrZotov mentioned this pull request Sep 8, 2023
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.

Drop Python 2 Support

3 participants