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

Support Python 3.6 to 3.10 #371

Merged
merged 13 commits into from
Feb 16, 2022
Merged

Support Python 3.6 to 3.10 #371

merged 13 commits into from
Feb 16, 2022

Conversation

WhyNotHugo
Copy link
Contributor

@WhyNotHugo WhyNotHugo commented Feb 15, 2022

Rebase of #325. Looks like pushing to stale branches does not update the existing PR.

Breaking change

  • Python versions which are past their EOL are no long supported.-

Proposed change

Drop support for Python versions which no longer have any upstream support. Improve forward compatibility up to Python 3.10.

This also should reduce dependency on older libraries, some of which are still pinned at old, insecure versions.

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New service (thank you!)
  • New feature (which adds functionality to an existing service)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests
  • Documentation or code sample

Example of code:

n/a

Additional information

Fixes #289
Fixes #290
Closes #303
Closes #325

Checklist

  • The code change is tested and works locally.
  • Local tests pass. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works. (kinda, CI runs on newer pythons).

If user exposed functionality or configuration variables are added/changed:

  • Documentation added/updated to README

Quentame and others added 5 commits February 15, 2022 19:34
Drop support of Python < 3.6
- remove six + future deps + usage
- update deps
- remove encoding
- remove (object)
@WhyNotHugo WhyNotHugo mentioned this pull request Feb 15, 2022
12 tasks
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
pyicloud/base.py Outdated Show resolved Hide resolved
README.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@balloob balloob left a comment

Choose a reason for hiding this comment

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

Looks great!

pyicloud/services/drive.py Outdated Show resolved Hide resolved
pyicloud/services/ubiquity.py Outdated Show resolved Hide resolved
@@ -15,7 +17,6 @@ commands =
{envbindir}/pytest

[testenv:lint]
basepython = python3
Copy link
Contributor

Choose a reason for hiding this comment

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

Please keep this line.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This really is a no-op nowadays, since the only supported Python is v3.

Copy link
Contributor

Choose a reason for hiding this comment

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

The tox docs aren't totally clear exactly how the default python is set. It says system python. Not all systems have an executable under python when Python 3 is installed, including my own system, but it still works for me to remove this line.

https://tox.wiki/en/latest/example/general.html#basepython-defaults-overriding

Ok, let's remove it for now.

pyicloud/base.py Outdated Show resolved Hide resolved
@MartinHjelmare
Copy link
Contributor

There are quite many pylint warnings for consider-using-f-string left. I think we should consider disabling that warning globally for now and solve that in a separate PR.

Hugo and others added 5 commits February 16, 2022 13:32
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
Co-authored-by: Martin Hjelmare <marhje52@gmail.com>
@MartinHjelmare
Copy link
Contributor

There are some lint warnings left to either resolve or disable and then we need to run black again with the updated version.

@balloob
Copy link
Collaborator

balloob commented Feb 16, 2022

Thanks! ❤️ 🎉

@balloob balloob merged commit 592ff46 into picklepete:master Feb 16, 2022
Copy link
Contributor

@MartinHjelmare MartinHjelmare left a comment

Choose a reason for hiding this comment

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

Just noting some things for a future clean up.

@@ -6,7 +6,7 @@


class AccountServiceTest(TestCase):
""""Account service tests"""
""" "Account service tests"""
Copy link
Contributor

Choose a reason for hiding this comment

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

Extra quote leftover.

@@ -6,7 +6,7 @@

# pylint: disable=pointless-statement
class DriveServiceTest(TestCase):
""""Drive service tests"""
""" "Drive service tests"""
Copy link
Contributor

Choose a reason for hiding this comment

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

As above.

@@ -5,7 +5,7 @@


class FindMyiPhoneServiceTest(TestCase):
""""Find My iPhone service tests"""
""" "Find My iPhone service tests"""
Copy link
Contributor

Choose a reason for hiding this comment

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

As above.

@balloob
Copy link
Collaborator

balloob commented Feb 16, 2022

Fixed final comments in 8c7ba2a

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants