-
Notifications
You must be signed in to change notification settings - Fork 6
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
refactor: DRY version #75
base: master
Are you sure you want to change the base?
refactor: DRY version #75
Conversation
tululbot/commands.py
Outdated
@@ -55,11 +55,12 @@ def quote(message): | |||
def who(message): | |||
app.logger.debug('Detected who command {!r}'.format(message.text)) | |||
about_text = ( | |||
'TululBot v1.11.2\n\n' | |||
'TululBot {!r}\n\n' |
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.
The !r
is not needed.
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.
OK
tests/test_commands.py
Outdated
@@ -110,11 +112,12 @@ def test_who(fake_message, mocker): | |||
who(fake_message) | |||
|
|||
expected_text = ( | |||
'TululBot v1.11.2\n\n' | |||
'TululBot {!r}\n\n' |
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.
The !r
is not needed.
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.
OK
tululbot/version.py
Outdated
VERSION = '1.11.3' | ||
|
||
|
||
def version_formatted(): |
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.
Since you can import VERSION
directly, this function is not really needed isn't it? At this point, the function only preprends v
to the version number. Also, this variable might be best put under tululbot/__init__.py
file so that it can be imported like from tululbot import VERSION
.
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 know we can import a constant. Thanks
Hey team i tried to change to what @kmkurn suggest, but the build was failed https://travis-ci.org/tulul/tululbot/builds/256555060. Can somebody help me and point out where is the mistake? Previous build worked (when use standalone VERSION file). |
@wazaundtechnik Sorry I think that's probably because of circular import or sth. I'm not sure either because I don't check myself. What if you put |
Do not hard code version into commands. Put it into separate file and let another module calls that command to retrieve version information.