-
Notifications
You must be signed in to change notification settings - Fork 103
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
DefaultConfigMixin: Modify implementation #597
Conversation
tests/spam_test.py
Outdated
@@ -38,8 +38,7 @@ def setUp(self): | |||
extra_config = { | |||
'DEFAULT_CONFIG': { | |||
'SpammingAlert': { | |||
'MAX_MSG_LEN': 500, | |||
'MAX_LINES': 5, | |||
'MAX_MSG_LEN': 50, |
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.
Why change from 500 to 50?
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.
misjudgement
Travis tests have failedHey @nvzard, 1st Buildpython -m pytest
|
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 Mixin's __init__
needs to assert that CONFIG_TEMPLATE
exists and is not empty.
It is quite common for mixin classes to create an empty class variable for things like this, so the Mixin can document the extra class variables it relies on.
I would like to see separate commits included in this PR for all of the configuration upgrades which we need (except for LabHub):
Any others? Only by putting them all in here together can we ensure that this is the last change needed to |
d46b5cc
to
c7f18e1
Compare
Comment on 5f8068a, file plugins/ghetto.py, line 20. Broken link - unable to connect to http://www.gizoogle.net/textilizer.php Origin: InvalidLinkBear, Section: |
c19c07f
to
e640c4c
Compare
71fca61
to
8db9a2a
Compare
Comment on a32333d, file plugins/ban.py, line 32. Broken link - unable to connect to https://api.gitter.im/v1/rooms Origin: InvalidLinkBear, Section: |
Comment on 8db9a2a, file tests/ghetto_test.py, line 13. Broken link - unable to connect to http://www.gizoogle.net/textilizer.php Origin: InvalidLinkBear, Section: |
Comment on 079e7bd, file plugins/ban.py, line 66. Broken link - unable to connect to https://api.gitter.im/v1/rooms Origin: InvalidLinkBear, Section: |
utils/mixin.py
Outdated
if default_config and not hasattr(self, 'config'): | ||
self.configure(default_config) | ||
if not hasattr(self, 'CONFIG_TEMPLATE'): # pragma: no cover | ||
self.log.error('CONFIG_TEMPLATE for plugin %s is missing.', self.name) |
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.
incorrect syntax!!
tests/answer_test.py
Outdated
@@ -24,6 +26,9 @@ def test_answer(self): | |||
self.assertIn('Please checkout the following links', self.pop_message()) | |||
self.push_message('!answer shell autocompletion') | |||
self.assertIn('Please checkout the following links', self.pop_message()) | |||
self.assertCommand('!plugin config answer', | |||
# Ignore InvalidLinkBear | |||
'{\'ANSWER_END\': \'http://0.0.0.0:8000\'}') |
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.
use self.answer_end_point , and dont # Ignore InvalidLinkBear
Store the default values in `CONFIG_TEMPLATE` and then allow a subset of those config variables overridden in the config.py's `DEFAULT_CONFIG`. Closes coala#596
Replace the use of environment variables with config variable. Closes coala#383
Replace the use of environment variables with config variable Closes coala#381
@gitmate-bot ff |
Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently |
@gitmate-bot ff |
Hey! I'm GitMate.io! This pull request is being fastforwarded automatically. Please DO NOT push while fastforward is in progress or your changes would be lost permanently |
Automated fastforward with GitMate.io was successful! 🎉 |
Store the default values in
CONFIG_TEMPLATE
andthen allow a subset of those config variables
overridden in the config.py's
DEFAULT_CONFIG
.Closes #596