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

RFC/WIP: adopt ruff and its code upgrade/format methods #437

Draft
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

RonnyPfannschmidt
Copy link

this starts with the isort and pyugprade rules + flake8 based rules
some warnings are resolved - about 141 more to go

i stumbled across a number of leftovers from historic python2 support

i'd like to get some input on whether adding type annotations would be appreciated or frowned upon
i'd like to get input on whether this type of project modernization would be appreciated or not before digging into the leftover linter warnings

@tobixen
Copy link
Member

tobixen commented Sep 25, 2024

I haven't looked through all of it, but I give it thumbs up. I should probably modernize my python skills myself one of those days :-)

@RonnyPfannschmidt
Copy link
Author

lovely, i'll gradually work trough the rest then as i want to use the library for modification of caldav calendars and want to give back

@tobixen
Copy link
Member

tobixen commented Sep 26, 2024

... but I should add, I find backward compatibility very important, as well as support for old python versions. I even believe it's a nice thing to support python versions that are already officially "EOL", as those python versions often are still included in distros that are not EOL. Though, python2 is officially not supported anymore, and there was a pull request removing python 3.6 from the test matrix slipping through some weeks ago :-)

@RonnyPfannschmidt
Copy link
Author

While personally I prefer to shed dead snakes as dropping support code and adopting new features tends to clean up and Speed up code, I'm absolutely on board with keeping them for projects that consider them important

In those cases I strongly appreciate a definition of the intended support range in order to avoid endless support for dead snakes (I love the win some of the newer versions enable)

@tobixen
Copy link
Member

tobixen commented Sep 27, 2024

Here are my thoughts:

  • As a bare minimum, all python versions that are officially supported by upstream should be supported
  • Shedding of old python version should be well announced in the CHANGELOG.md file
  • Preferably, shedding of old python versions should only be done when changing the major version number of the library (I did have great plans for the 2.0-version of this library, but perhaps I will need to procrastinate it until 8.0 or something) :-)
  • It's a nice thing to keep an overview of python versions used by the popular Linux distros that aren't EOL'ed yet, and consider that before shedding support for old python versions
  • We try to avoid "schrödingers support". It's a nice thing to toss a warning or error if the python version is not officially supported, rather than something suddenly breaking out of the blue in a production environment.

@tobixen
Copy link
Member

tobixen commented Oct 26, 2024

So this will go into a 2.0-release, which will officially break with 3.7 (and possibly 3.8 as well). I cannot give any ETA on a 2.0-release currently, but possibly in some few months.

@tobixen tobixen added this to the v2.0 milestone Oct 26, 2024
@tobixen
Copy link
Member

tobixen commented Nov 5, 2024

Now that 1.4.0 has been released, we may consider shedding support for python 3.7 and python 3.8 and work towards a 2.0-release.

There are references to python 3.7 in docs/source/index.rst, pyproject.toml and .github/workflows/tests.yaml. If those are fixed, I believe the tests will pass, and this work may be merged into main.

Post-edit: there is also tox.ini.

@RonnyPfannschmidt RonnyPfannschmidt force-pushed the ruff-pyupgrade-setuptools-upgrades branch from 1652ece to 47110ed Compare November 6, 2024 10:20
@RonnyPfannschmidt
Copy link
Author

i started picking this up again - there are still dozens of warnings, some dead code and more

@RonnyPfannschmidt
Copy link
Author

im particularly confused around the numbered variables in tests, of which some are unused. some are missed all together

@tobixen
Copy link
Member

tobixen commented Nov 6, 2024

im particularly confused around the numbered variables in tests, of which some are unused. some are missed all together

The functional tests are more or less a complete mess, developed organically over time. I've considered doing something with it, but ... it's not a high priority. An idea could be to rename it into test_legacy_ftest.py or something, and then make new test code from scratch (and deleting the legacy tests once the new tests are well enough done).

One of the problems with the tests is that many of them are not really testing the library code per se, but rather the servers. I started today writing up some new code in the test directory, check_server_compatibility.py, I will push it in a separate branch as soon as it's doing anything useful. The purpose is to make a tool that can be run from the command line towards any server and spit out a list of "quirks". The tests today work the other way around, one have to provide the expected "quirks" and then the tests will break if the list is incorrect.

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.

2 participants