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

use py3 instead py2 #3153

Merged
merged 2 commits into from
Mar 3, 2020
Merged

use py3 instead py2 #3153

merged 2 commits into from
Mar 3, 2020

Conversation

jnozsc
Copy link
Contributor

@jnozsc jnozsc commented Feb 22, 2020

Signed-off-by: jnozsc jnozsc@gmail.com

By submitting this pull request, I confirm the following:
please fill any appropriate checkboxes, e.g: [X]

  • I have read and understood the contributors guide, as well as this entire template.
  • I have made only one major change in my proposed changes.
  • I have commented my proposed changes within the code.
  • I have tested my proposed changes, and have included unit tests where possible.
  • I am willing to help maintain this change if there are issues with it later.
  • I give this submission freely and claim no ownership.
  • It is compatible with the EUPL 1.2 license
  • I have squashed any insignificant commits. (git rebase)

Please make sure you Sign Off all commits. Pi-hole enforces the DCO.


What does this PR aim to accomplish?:
replace usage of python2

How does this PR accomplish the above?:
use python3

What documentation changes (if any) are needed to support this PR?:
NA


  • You must follow the template instructions. Failure to do so will result in your pull request being closed.
  • Please respect that Pi-hole is developed by volunteers, who can only reply in their spare time.

Signed-off-by: jnozsc <jnozsc@gmail.com>
@dschaper
Copy link
Member

dschaper commented Mar 2, 2020

This looks good to me, just not sure if this needs to be in release/v5.0 or development.

@dschaper
Copy link
Member

dschaper commented Mar 2, 2020

Would 3.4 or 3.6 work as well? I don't see anything that would prevent those. Travis defaults to 3.6 so I'm comfortable with their tooling to work with that version.

@jnozsc
Copy link
Contributor Author

jnozsc commented Mar 2, 2020

I recommend to use the latest version, python3.8, so that people don't have to upgrade from py3.x to py3.y in any time next year. It is just personal preference.

https://devguide.python.org/#status-of-python-branches

About python 3.4 or python 3.6 compatibly. I think it should work with any python 3 version. But I do notice some packages will/is drop(ing) 3.4 support since it has already EOL ( https://www.python.org/downloads/release/python-3410/ )

@dschaper
Copy link
Member

dschaper commented Mar 2, 2020

I'd like 3.6. The reason being that if we say 3.8 then we are making a contract to support any features that may be in 3.8. Ubuntu 18 ships with 3.6, travis is default 3.6 (though you can change that of course). I'd prefer to have it be the lowest possible version required so the largest number of people can contribute to it.

Signed-off-by: jnozsc <jnozsc@gmail.com>
@jnozsc
Copy link
Contributor Author

jnozsc commented Mar 2, 2020

ok, fixed in 49fc265

@dschaper
Copy link
Member

dschaper commented Mar 3, 2020

@PromoFaux Any preference on the branch this should merge in to?

dschaper
dschaper previously approved these changes Mar 3, 2020
Copy link
Member

@dschaper dschaper left a comment

Choose a reason for hiding this comment

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

Approved pending branch choice.

@PromoFaux
Copy link
Member

PromoFaux commented Mar 3, 2020

Either is fine I think. The only python we have is in the tests, so it's not actually going to affect anything in userland, and no real rush to get it "released"

@dschaper
Copy link
Member

dschaper commented Mar 3, 2020

For safety sake I think development would be better. Release 5 is very close and I don't want to throw something in there that may cause unintended issues.

jnozsc, can you send this to development instead of release/v5.0 and I'll merge it.

@jnozsc jnozsc dismissed dschaper’s stale review March 3, 2020 07:28

The base branch was changed.

@jnozsc jnozsc changed the base branch from release/v5.0 to development March 3, 2020 07:28
@jnozsc
Copy link
Contributor Author

jnozsc commented Mar 3, 2020

changed. thanks all

@dschaper dschaper merged commit 4a71134 into pi-hole:development Mar 3, 2020
@jnozsc jnozsc deleted the sunset-py27 branch March 3, 2020 07:37
@PromoFaux PromoFaux mentioned this pull request Jul 5, 2020
@pralor-bot
Copy link

This pull request has been mentioned on Pi-hole Userspace. There might be relevant details there:

https://discourse.pi-hole.net/t/pi-hole-5-1-released/35577/1

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.

None yet

4 participants