collective / collective.recipe.omelette

https://pypi.org/project/collective.recipe.omelette/
4 stars 1 forks source link

py3 #11

Closed goschtl closed 2 years ago

goschtl commented 2 years ago

Hi,

i changed to new setuptools zopetesting. py3 according to this issue

https://github.com/collective/collective.recipe.omelette/issues/10

mauritsvanrees commented 2 years ago

Thanks for working on this!

This is hard to review because there are lots of changes. Most are boring (but useful), like black. Within those boring bits I need to look at the few interesting changes. When all is in one big commit, it is highly likely that a reviewer overlooks an error. Can you split this in multiple commits, please?

Also, you have access to the collective, so can you create a branch here? That makes it easier to try it out, without having to add another remote.

goschtl commented 2 years ago

Hey,

yes i can do that. I question which comes to my mind. This version should it work for py2 and py3? Or would it be ok to drop py2 support?

Christian

Am Di., 16. Nov. 2021 um 12:52 Uhr schrieb Maurits van Rees < @.***>:

Thanks for working on this!

This is hard to review because there are lots of changes. Most are boring (but useful), like black. Within those boring bits I need to look at the few interesting changes. When all is in one big commit, it is highly likely that a reviewer overlooks an error. Can you split this in multiple commits, please?

Also, you have access to the collective, so can you create a branch here? That makes it easier to try it out, without having to add another remote.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/collective/collective.recipe.omelette/pull/11#issuecomment-970196573, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABK7M3KDHSVX5VSQJUWP23UMJAYNANCNFSM5IBFAPBA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

-- Mit freundlichen Grüßen Christian Klinger @.*** 0172-8139138

mauritsvanrees commented 2 years ago

I would say: preferably Python 2 and 3 compatible. But if it is too hard, Py 3-only would be fine.

On the other hand, this package has seen a few changes in summer last year, and the last change before that was in 2014. So we can say this package is pretty stable on Python 2, and unlikely to need bug fixes for that. So dropping Python 2 support actually seems fine to me, if that helps you.

davisagli commented 2 years ago

+1 from me for dropping py2 support (with a major version bump)

goschtl commented 2 years ago

Something related but different... Do you know a tool which does the same that collective.recipe.omelette for normal python venv's too?

mauritsvanrees commented 2 years ago

Something related but different... Do you know a tool which does the same that collective.recipe.omelette for normal python venv's too?

I was wondering about this too, recently. I think for virtualenvs we don't need anything special. You can just look in lib/python*/site-packages/

mauritsvanrees commented 2 years ago

Superseded by your PR #12.