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
Fixed manual page description #155
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'm definitely for updating that string, but I think it'll be better to just hard code the string there. It's not like it's going to change much (if ever).
I also think we need to fix the commit message. I normally wouldn't complain about nits like this (instead I'd just fix them in a follow up change) but I'm not able to edit the message after it's merged. So can you clean up the commit message a bit. Besides the spelling error having a description of what and why the change is needed would be good. https://wiki.openstack.org/wiki/GitCommitMessages is a good guide on writing commit messages (and the reasons why it's important even for simple changes)
stestr/cli.py
Outdated
from stestr import version | ||
|
||
__version__ = version.version_info.version_string_with_vcs() | ||
__description__ = util.cfg_to_args()['description'] |
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.
Will this work? Looking at the code in https://github.com/openstack-dev/pbr/blob/master/pbr/util.py#L195 it wants the actual setup.cfg file path. The default will work if you're in the root of the repo, but what if you run stestr --help from any other directory I don't think this will work. You might be able to use pkg_resources to pull the description from the sdist after stestr is installed, but that's kinda costly and I think just hard coding the string might be easier.
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.
Yup, it appears to be costly, It is also breaking the tests. I will hard code the description.
In the stestr MANUAL page, the description of the tool was 'stestr application' coming from stestr/cli.py, which is wrong. It should be the tool description which is 'A parallel Python test runner built around subunit'. It fixes the same.
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.
Thanks for updating. LGTM
No description provided.