Skip to content

Conversation

@kloczek
Copy link
Contributor

@kloczek kloczek commented May 29, 2024

Accordint to https://endoflife.date/python python 3.7 as been EOSed 27 Jun 2023.
Fileter all python code over `pupgrade --py38-plus'.

kloczek added 2 commits May 29, 2024 00:07
Accordint to https://endoflife.date/python python 3.7 as been EOSed
27 Jun 2023.
Fileter all python code over `pupgrade --py38-plus'.

Signed-off-by: Tomasz Kłoczko <kloczek@github.com>
Mostly drops unused impors and other automated cleanups.

Signed-off-by: Tomasz Kłoczko <kloczek@github.com>
@firewave
Copy link
Collaborator

Thanks for your contribution.

We are intentionally supporting those EOL Python versions. The oldest platform we are targeting is Ubuntu 16.04 and that comes with Python 3.5.

But there are some modernization in these changes which could be applied as they are supported by that version. Also there are some changes I do not like in general which I will comment on separately.

all_files = args.dumpfile
if args.file_list:
with open(args.file_list, 'rt') as f:
with open(args.file_list) as f:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I do not like this at all. This should be explicitly specified for various reasons.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was considering adding pyupgrade to the CI but the open() thing still irked me.

And then I came across asottile/pyupgrade#714.

Given the attitude of the author I will refrain from integrating that tool and would also be reluctant to accept changes which are based on it. Such behavior should be not supported...

# dumpfile ends with ".dump"
ctu_info_file = dumpfile[:-4] + "ctu-info"
with open(ctu_info_file, 'at') as f:
with open(ctu_info_file, 'a') as f:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same as above. "text" vs "binary" might be important in some cases so that should be explicitly stated.

@firewave
Copy link
Collaborator

An additional note - the CI passes because we are not testing all the parts on those older platform. That was done in the scriptcheck.yml workflow but as those versions no longer work when freshly installed as non-distro supported versions we had to disable it. That needs to be re-added in a different way to the docker-based jobs but I didn't get around to it yet.

@firewave
Copy link
Collaborator

See #3596 about properly fixating, testing and documenting the targeted platforms.

@kloczek
Copy link
Contributor Author

kloczek commented May 29, 2024

We are intentionally supporting those EOL Python versions. The oldest platform we are targeting is Ubuntu 16.04 and that comes with Python 3.5.

Using latest cppcheck on so old version does not make to much sense.
Isn't it? 🤔
Last cppcheck supported by 16.04 is 1.72.
https://launchpad.net/ubuntu/xenial/+package/cppcheck
Update policy for that version says only about fixing only critical/security bugs.
Update to latest 2.x could not fit in such criteria.

@firewave
Copy link
Collaborator

Using latest cppcheck on so old version does not make to much sense.
Isn't it? 🤔
Last cppcheck supported by 16.04 is 1.72.
https://launchpad.net/ubuntu/xenial/+package/cppcheck

That is exactly why we need to support it, because people have to do their own build to get the latest version.

This could also be mitigated by offering something like an up-to-date (and especially official) Snapcraft package but that is not available on all distros.

@firewave
Copy link
Collaborator

What is pupgrade? Did you mean https://github.com/asottile/pyupgrade?

Also we still support Python 2.7. But that will be removed in 2.16 so we cannot make any changes until then.

@kloczek
Copy link
Contributor Author

kloczek commented May 29, 2024

What is pupgrade? Did you mean https://github.com/asottile/pyupgrade?

Yes .. typo.

Also we still support Python 2.7. But that will be removed in 2.16 so we cannot make any changes until then.

AFAIK there is no even single still supported distro which still uses python 2.7.
Python 2.7 has been EOSed in 2020 (in few days will be 4 years ago).

@firewave
Copy link
Collaborator

AFAIK there is no even single still supported distro which still uses python 2.7.
Python 2.7 has been EOSed in 2020 (in few days will be 4 years ago).

We still had CentOS 7 support a while ago. And we are very few people some things might not be taken care of so Python 2.7 wasn't deprecated until recently.

I think most of these changes can also be detected by pylint. We have that disabled in the CI but I will try to enable it again.

@kloczek
Copy link
Contributor Author

kloczek commented May 29, 2024

Please check which one version of the cppcheck is in CentOS 7.
If it would be any critical issue probably it will be resolved by apply some patch instead upgrade to latest version.

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