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
Make python 3.6 the minimum version #2788
Conversation
0b2db96
to
dbe1bd8
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, @ulyssessouza. I made some comments.
Have a nice day :)
@@ -1,11 +1,7 @@ | |||
ARG PYTHON_VERSION=2.7 | |||
ARG PYTHON_VERSION=3.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that be 3.6
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For this one, it's the version on which it will be compiled. Doesn't have a relationship with the minimal version
@@ -73,7 +70,7 @@ def runTests = { Map settings -> | |||
throw new Exception("Need Docker version to test, e.g.: `runTests(dockerVersion: '19.03.12')`") | |||
} | |||
if (!pythonVersion) { | |||
throw new Exception("Need Python version being tested, e.g.: `runTests(pythonVersion: 'py2.7')`") | |||
throw new Exception("Need Python version being tested, e.g.: `runTests(pythonVersion: 'py3.7')`") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here too 3.6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This one is the same as above. It's the version on which it will be tested. Not released.
tests/Dockerfile-dind-certs
Outdated
@@ -1,4 +1,4 @@ | |||
ARG PYTHON_VERSION=2.7 | |||
ARG PYTHON_VERSION=3.7 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same question as Felipe, + CI expected green build must be edited as currently the PR notifications list expected builds 3.5, 2.7
dbe1bd8
to
116ab9c
Compare
Signed-off-by: Ulysses Souza <ulyssessouza@gmail.com>
116ab9c
to
c8fba21
Compare
@gtardif For the CI question, I'm not sure from where it comes. That's maybe cached from previous build where it used to exist on github actions configuration. Since this PR removes it |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Resolves #2767