search

teemtee / fmf

Flexible Metadata Format
GNU General Public License v2.0
22 stars 28 forks source link

Drop support for Python 2 #144

Closed psss closed 3 years ago

psss commented 3 years ago

Remove related stuff from the spec file. Remove Python 2 only code, disable epel-7 packit. Remove irrelevant adjust rule from unit tests. Get rid of other unicode related leftovers. Use default encoding for running commands. Explicitly mention the utf-8 encoding in the docs. Remove unnecessary object from class definitions.

psss commented 3 years ago

@lachmanfrantisek, I've removed epel-7 from the .packit.yaml but the epel-7 build seems to be included in the latest packit job. Is that expected?

lachmanfrantisek commented 3 years ago

@lachmanfrantisek, I've removed epel-7 from the .packit.yaml but the epel-7 build seems to be included in the latest packit job. Is that expected?

I thought we had already fixed this. (@mfocko I recall we've discussed this).

@psss It should affect only this PR and will not be used once merged...

mfocko commented 3 years ago

@psss It's twice there, once for pull_request trigger and second time for builds on push to master (← this one I see removed in diff for PR).

We've also changed the behavior, such that we remove targets only on projects created and maintained by Packit user, so if you want to remove epel-7 from the Copr repo you own (where you build everything pushed to master), you need to do so yourself. However it shouldn't build for it, cause it checks only if the targets from config are enabled and builds those. I won't mind if you check it ;)

psss commented 3 years ago

@lachmanfrantisek, @mfocko, thanks for quick review and hints. I've completely forgotten that the target config is there twice. After removing the second one everything seems to be working as expected. Sorry for the noise.

lukaszachy commented 3 years ago

Good point @jscotka , unicode(item) raises NameError: for python3 so except cause will be hit each time

psss commented 3 years ago

@lukaszachy, @jscotka, good points! All should be covered now.

psss commented 3 years ago

I think that also "class Tree(object)" should not be used inside python3 just "class Tree:"

@jscotka could you please elaborate a bit more why object should not be used?

jscotka commented 3 years ago

I think that also "class Tree(object)" should not be used inside python3 just "class Tree:"

@jscotka could you please elaborate a bit more why object should not be used?

I had that feeling, maybe there were some hints or docs what I've read that somewhere, now when looking inside: https://stackoverflow.com/questions/4015417/why-do-python-classes-inherit-object it seems that there were difference inside Python2, and in python3 there syntax class X: and class X(object): is totally equivalent, so that reason why to not use it is that you saves 8 chars.

psss commented 3 years ago

@jscotka, thanks for clarification. Unnecessary object strings removed from class definitions. @lukaszachy anything else to change or are we ready for squash and merge?

lukaszachy commented 3 years ago

LGTM.

psss commented 3 years ago

@lukaszachy, @jscotka, @FrNecas, thanks much for the review! Rebased, squashed, now waiting for tests.