-
Notifications
You must be signed in to change notification settings - Fork 128
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
New config option buildconfig.test_version_suffix
#502
New config option buildconfig.test_version_suffix
#502
Conversation
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.
I didn't test it in action but the code looks good to me. Except for that one EPEL7 thing. Thank you very much for the PR.
m = re.match(r'^(\s*Version:\s*)(.+?)\s*$', line) | ||
if m: | ||
print(f"{m.group(1)}{m.group(2)}{version_suffix}") | ||
continue |
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.
I don't know, do we still care about EPEL7? Anyway, it's easy not to break the backward compatibility here.
Compiling /builddir/build/BUILDROOT/tito-0.6.26-1.20240927180525922719.pr502.2.g9069fc1.el7_9.x86_64/usr/lib/python2.7/site-packages/tito/common.py ...
File "/usr/lib/python2.7/site-packages/tito/common.py", line 611
print(f"{m.group(1)}{m.group(2)}{version_suffix}")
^
SyntaxError: invalid syntax
Can you please use the .format
syntax?
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.
I don't know, do we still care about EPEL7?
EPEL 7 is EOL, so we can't update in Koji -> only chance is to ship in Copr now. IMVHO good time to forget about Python 2 support...
Can you please use the .format syntax?
Can do, if you prefer 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.
We don't even need .format()
in this case, used plain +
.
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.
IMVHO good time to forget about Python 2 support...
I have no problem if we make that decision, drop EPEL7 from tests and so on. I just didn't want to break it by adding one unnecessary f-string :-)
If configured like $ cat .tito/tito.props [buildconfig] test_version_suffix = .tito.git The `tito build --test ...` NEVRA is always higher than the NEVRA of released RPMs. Fixes: rpm-software-management#460
9069fc1
to
2860fef
Compare
If configured like
The
tito build --test ...
NEVRA is always higher than the NEVRA of released RPMs.Fixes: #460