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 upstream_tag_template to get version from tag #959
Conversation
Congratulations! One of the builds has completed. 🍾 You can install the built RPMs by following these steps:
Please note that the RPMs should be used only in a testing environment. |
This comment has been minimized.
This comment has been minimized.
Build succeeded.
|
As discussed on IRC this PR fixed your issue |
Thank you a lot. I will finally be able to remove the workarounds we have ;) |
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.
Is this still WIP?
the code part is finished, only integration tests and documentation is missing |
packit/upstream.py
Outdated
# raw_version - version before processing by Upstream.get_version_from_tag() | ||
raw_ver = self.command_handler.run_command( |
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.
Not sure if the comment gives us any new value.
# raw_version - version before processing by Upstream.get_version_from_tag() | |
raw_ver = self.command_handler.run_command( | |
raw_ver = self.command_handler.run_command( |
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.
fixed
self.package_config.current_version_command, | ||
return_output=True, | ||
cwd=self.local_project.working_dir, | ||
).strip() | ||
logger.debug(f"Version: {ver}") |
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.
Don't we want to log it? Or both?
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.
fixed
packit/upstream.py
Outdated
try: | ||
return p.match(tag).group(field) | ||
except (IndexError, AttributeError): | ||
logger.error( | ||
f'Unable to extract "{field}" from {tag} using ' | ||
f"{self.package_config.upstream_tag_template}" | ||
) | ||
raise |
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 am looking for the time we can be python>=3.8 only and use Assignment Expressions.
But I would still try to avoid unnecessary try-except
when not needed...
try: | |
return p.match(tag).group(field) | |
except (IndexError, AttributeError): | |
logger.error( | |
f'Unable to extract "{field}" from {tag} using ' | |
f"{self.package_config.upstream_tag_template}" | |
) | |
raise | |
match = p.match(tag) | |
if match and field in match.groups(): | |
return match.group(field) | |
else: | |
msg = ( | |
f'Unable to extract "{field}" from {tag} using ' | |
f"{self.package_config.upstream_tag_template}" | |
) | |
logger.error(msg) | |
raise PackitException(msg) |
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.
fixed
why you prefer if...else over try...except when dealing with unwanted errors?
tests/unit/test_upstream.py
Outdated
mock = flexmock(synced_files=None, upstream_package_name=upstream_package_name) | ||
mock.should_receive("current_version_command") | ||
return mock |
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.
You can use mock
method to get the original flexmock
instance:
mock = flexmock(synced_files=None, upstream_package_name=upstream_package_name) | |
mock.should_receive("current_version_command") | |
return mock | |
return flexmock( | |
synced_files=None, | |
upstream_package_name=upstream_package_name | |
).should_receive("current_version_command").mock() |
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.
fixed
Build failed.
|
recheck |
Build failed.
|
recheck |
Build failed.
|
Use package config variable upstream_tag_template to extract version from git tag.
Build failed.
|
recheck |
1 similar comment
recheck |
Build failed.
|
recheck |
1 similar comment
recheck |
Build failed.
|
recheck |
Build failed.
|
@jkonecny12 |
Use package config variable upstream_tag_template to extract version
from git tag.
TODO: