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

remove Python2 crumbs #541

Merged
merged 7 commits into from
Jul 24, 2024
Merged

remove Python2 crumbs #541

merged 7 commits into from
Jul 24, 2024

Conversation

a-detiste
Copy link
Contributor

Hi !

I saw this new package in Debian today popping up in my QA list.

Here's a cleanup of Python2 code.

@emollier : please remove python3-six dependency when a new release is published, or I'll do if I notice first

https://tracker.debian.org/pkg/python-nixio

https://salsa.debian.org/detiste-guest/python3-six-removal/-/commit/b3218f61adb31c09cac2de735fd83997abe9b707

@emollier
Copy link
Contributor

Hi Alexandre,

Thanks for your QA work, I'll try to not forget about the python3-six package once removed upstream; I left a Fixme item in the d/control file so lintian will ring some alarm bells for me when the time will come.

Have a nice day, :)
Étienne.

@a-detiste
Copy link
Contributor Author

a-detiste commented Apr 28, 2024

I discoverd this feature :-) (checking only "Release")

image

But messing with UDD & postgres is fun too

jgrewe
jgrewe previously approved these changes Apr 29, 2024
Copy link
Member

@jgrewe jgrewe left a comment

Choose a reason for hiding this comment

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

lgtm, thanks!

uh, seems I was a little too fast with my approval...

Copy link
Member

@achilleas-k achilleas-k left a comment

Choose a reason for hiding this comment

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

Thanks for bringing this to our attention and doing a lot of the work to clean it up. There appear to be a few issues with iterating on a single type (see inline comment).

I'd also love to see this PR split into multiple commits if it's not too much trouble. For example, the changes in setup.py could be their own commit.

I understand if you don't have time to work on this and perhaps this was meant as a quick patch to make the project buildable for the distro and bring it up to date a bit, so let us know and we can take it from here if necessary.

nixio/block.py Outdated Show resolved Hide resolved
@a-detiste
Copy link
Contributor Author

I'm out for the day :-) You can merge as-is if you like it of course. six.ensure_str() & six.ensure_text() are always bit more complicated to replace.

@a-detiste a-detiste requested a review from achilleas-k May 5, 2024 20:14
@a-detiste
Copy link
Contributor Author

I on Holliday for one week. Do you still need my help before merging this ?

@coveralls
Copy link

Coverage Status

coverage: 79.718% (-0.06%) from 79.774%
when pulling 9f3d24c on a-detiste:master
into 915a232 on G-Node:master.

@jgrewe
Copy link
Member

jgrewe commented Jul 9, 2024

Hi @a-detiste, just enjoy your holidays :)
Let me poke @achilleas-k to draw his attention to this pr again.
Thanks again.

@achilleas-k achilleas-k merged commit 444aae5 into G-Node:master Jul 24, 2024
22 checks passed
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.

5 participants