doonze / EmbyUpdate

A python script for automatically updating Emby to the latest version on Linux distros.
GNU Lesser General Public License v3.0
15 stars 3 forks source link

Pep fixes #7

Closed deathbybandaid closed 2 years ago

deathbybandaid commented 2 years ago

I have intentions of future PRs, and step one was cleaning up some PEP errors.

This also introduces some better handling of ConfigParser for python 2/3. (realistically py2 support should be killed off.)

doonze commented 2 years ago

I'll try to review these this week. I'm all for PEP improvements, though I did have a personal preference reason to have the version number before the imports. I know it's not PEP, but I liked that the very first thing I saw was the version I was on. Reminded me to update it! LOL

Originally I wrote this when all distro's still only installed Python 2 by default. I also wrote it in nano originally, so no IDE. More or less, only power users/programers were installing Python 3 when this was first developed. It was also my first major Python project. I've since started using IDE's and written 10,000's of thousands of lines of Python 3 code.

I truly want to phase out Python 2, but at this point there is little reason, unless I want to add a new feature only offered in 3. And there are still MANY MANY installations with only P2 installed on them. I'm not worried about new users. I'm all for making people update to Python 3 to use the script. But I don't want to break it for people who already installed it and are still on P2. They wouldn't even be aware it's broken for who knows how long.

That being said. as long as core updates still worked on P2, any new optional features, I had planned on writing for P3 only. I just don't want anyone on P2 left with a broken script when it auto updates.

Also, have you tested your PEP changes on P2? It gets finicky about indents, and P3 and P2 don't play nice all the time there.

doonze commented 2 years ago

Also, do you want to be credited for you contributions by your github use id or real name (if you want to provide?)

doonze commented 2 years ago

Turns out there was a fatal bug in the configphaser in this pull request. Unfortunately, I pushed the change and broke all installed instances of the script to the point they can no longer selfupdate and are broken until manually updated.