Fix logic for detecting if Python2 is supported #446
Conversation
| @@ -21,7 +21,7 @@ | |||
| %{!?__python3: %global __python3 python3} | |||
| %{!?python3_shortver: %global python3_shortver %(%{__python3} -c 'import sys; print(str(sys.version_info.major) + "." + str(sys.version_info.minor))')} | |||
|
|
|||
| %if ! 0%{?rhel} >= 8 && ! 0%{?suse_version} > 1550 | |||
| %if 0%{?rhel} < 8 && 0%{?suse_version} <= 1500 | |||
There was a problem hiding this comment.
Matt in the issue only inverted the equality, but you also changed the suse version from 1550 to 1500 -- is that on purpose?
(might want to say that in the commit message if so)
There was a problem hiding this comment.
It was unintentional - I don't have OpenSuSE so I'm not sure exactly what version stopped supporting py2. Changing the 1500 back to 1550 is probably the right thing to do.
There was a problem hiding this comment.
%suse_version 1500 includes Leap/SLE 15 - which still has python2. %suse_version 1550 is Tumbleweed/Factory (currently) where python2 has been dropped from.
There was a problem hiding this comment.
Ok, so from that I take we don't want python2 in Tumbleweed (1550), so the new logic is correct -- wasn't that just what the problem is about?
Well, non-convoluted logic is better for everyone anyway, thanks!
|
@e4t the test failed for unrelated reason. Refreshing the commit would retrigger them. It could be good to update your commit message to follow the way we usually do them. See as an example: 23621a5#diff-8798411bf1e861edaa6a04787c30ade5 Basicly (feel free to adapt or reword): |
It's now less obfuscated. Python2 was conditionally enabled only if building for an old RHEL or old SLES but the version tests was not correct. Thanks to Matt Ezell from Oak Ridge. Fixes cea-hpc#445. Signed-off-by: Egbert Eich <eich@suse.com>
|
I've updated and pushed it again, now. |
|
Hi @e4t, thanks for the PR! LGTM. Could you please push your last change to the e4t/master branch so this PR gets updated? |
|
Done. I had accidentally created an additional branch. |
|
Thanks @e4t! |
This fixes Issue #445.
Thanks to Matt Ezell from Oak Ridge.
Signed-off-by: Egbert Eich eich@suse.com