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.

Sign up for GitHub

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

Jump to bottom

drop python<=3.7 support #170

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

drop python<=3.7 support #170

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cc1c134
drop python<=3.7 support
kloczek Sep 29, 2024
8556b7f
manually update setup.py
kloczek Sep 29, 2024
bbf672b
remove pytehon 3.7 from CI as well
kloczek Sep 29, 2024
5a396dc
remove Python >= 3.6 dependent code
kloczek Sep 29, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,11 @@ jobs:
fail-fast: true
matrix:
python-version:
- "3.7"
- "3.8"
- "3.9"
- "3.10"
- "3.11"
- "3.12"
- "pypy-3.7"
Copy link
Owner

Choose a reason for hiding this comment

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

Please add some newer PyPy version instead. I don't want to claim PyPy compatibility if I don't run tests on PyPy in CI.

vcs:
- bzr
- git
Expand Down
2 changes: 1 addition & 1 deletion check_manifest.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ def format_missing(missing_from_a, missing_from_b, name_a, name_b):

class CommandFailed(Failure):
def __init__(self, command: List[str], status: int, output: str) -> None:
super().__init__("%s failed (status %s):\n%s" % (
super().__init__("{} failed (status {}):\n{}".format(
command, status, output))


Expand Down
6 changes: 2 additions & 4 deletions setup.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,29 +43,27 @@
'License :: OSI Approved :: MIT License',
'Operating System :: OS Independent',
'Programming Language :: Python',
'Programming Language :: Python :: 3',
'Programming Language :: Python :: 3.7',
'Programming Language :: Python :: 3.8',
'Programming Language :: Python :: 3.9',
'Programming Language :: Python :: 3.10',
'Programming Language :: Python :: 3.11',
'Programming Language :: Python :: 3.12',
'Programming Language :: Python :: 3.13',
'Programming Language :: Python :: Implementation :: CPython',
'Programming Language :: Python :: Implementation :: PyPy',
],
license=licence,

py_modules=['check_manifest'],
zip_safe=False,
python_requires=">=3.7",
python_requires=">=3.8",
install_requires=[
'build>=0.1',
'setuptools',
'tomli;python_version < "3.11"',
],
extras_require={
'test': [
'mock >= 3.0.0; python_version == "3.7"',
'pytest',
'wheel',
],
Expand Down
31 changes: 3 additions & 28 deletions tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
import codecs
import locale
import os
import posixpath
Expand All @@ -18,27 +17,7 @@
from xml.etree import ElementTree as ET


if sys.version_info >= (3, 8):
from unittest import mock
else:
# unittest.mock in 3.7 is too old to support
# all the features used in the test suite
import mock

from check_manifest import rmtree

Copy link
Owner

Choose a reason for hiding this comment

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

This import is necessary and removing it breaks the test suite (which you can only see on Appveyor because GitHub Actions cancels all the useful test runs early after seeing the isort failure).


CAN_SKIP_TESTS = os.getenv('SKIP_NO_TESTS', '') == ''

Copy link
Owner

Choose a reason for hiding this comment

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

How do things not break if you remove this line? Something is wrong.


try:
codecs.lookup('oem')
except LookupError:
HAS_OEM_CODEC = False
else:
# Python >= 3.6 on Windows
HAS_OEM_CODEC = True

from unittest import mock

Copy link
Owner

Choose a reason for hiding this comment

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

This import needs to be moved above the from xml.etree import or isort will complain.

Also, I need to review my isort command-line because a complaint of the sort

ERROR: /home/runner/work/check-manifest/check-manifest/tests.py Imports are incorrectly sorted and/or formatted.

is useless when it won't tell me which imports where and how to sort them. It could show a diff or something.

class MockUI:

Expand Down Expand Up @@ -115,7 +94,7 @@ def test_run_no_such_program(self):
should_start_with = "could not run ['there-is-really-no-such-program']:"
self.assertTrue(
str(cm.exception).startswith(should_start_with),
'\n%r does not start with\n%r' % (str(cm.exception),
'\n{!r} does not start with\n{!r}'.format(str(cm.exception),
should_start_with))

Comment on lines 95 to 98
Copy link
Owner

Choose a reason for hiding this comment

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

This line is now misindented: it was matching the position of the ( in the previous line, and no longer does. (The pain of these indentation adjustments is one of the reasons I've lately switched to a more Black-inspired indentation style with a line break right after the opening ( and a 4-space increase of indentation on the next line.)

If we used an f-string here, maybe it would fit without wrapping?

def test_mkdtemp_readonly_files(self):
Expand Down Expand Up @@ -179,7 +158,7 @@ def test_copy_files(self):
'cp a %s' % n('/dest/dir/a'),
'mkdir %s' % n('/dest/dir/b'),
'makedirs %s' % n('/dest/dir/c/d'),
'cp %s %s' % (n('c/d/e'), n('/dest/dir/c/d/e')),
'cp {} {}'.format(n('c/d/e'), n('/dest/dir/c/d/e')),
])
Copy link
Owner

Choose a reason for hiding this comment

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

Okay, why change this line but leave all of the previous lines using %-formatting? This offends my sense of consistency.


def test_get_one_file_in(self):
Expand Down Expand Up @@ -1165,8 +1144,6 @@ class TestBzr(VCSMixin, unittest.TestCase):
vcs = BzrHelper()


@unittest.skipIf(HAS_OEM_CODEC,
"Python 3.6 lets us use 'oem' codec instead of guessing")
class TestBzrTerminalCharsetDetectionOnOldPythons(unittest.TestCase):

Copy link
Owner

Choose a reason for hiding this comment

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

I don't think these two test classes could possibly work at the same time. This one is meant for Python < 3.6 and should be removed entirely.

@mock.patch('sys.stdin')
Expand Down Expand Up @@ -1198,8 +1175,6 @@ def test_terminal_encoding_cp0(self, mock_stdout):
self.assertEqual(Bazaar._get_terminal_encoding(), None)


@unittest.skipIf(not HAS_OEM_CODEC,
"'oem' codec not available on Python before 3.6")
class TestBzrTerminalCharsetDetectionOnNewPythons(unittest.TestCase):

def test_terminal_encoding_cp0(self):
Expand Down
Loading