Skip to content
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

nevermind.py: Change regex pattern #606

Merged
merged 1 commit into from
Aug 7, 2018

Conversation

bhavya17037
Copy link
Contributor

Change regex pattern so that corobo only apologizes
when "corobo nm" or "corobo nevermind" are used.
Also add unit tests for the same

Fixes #519

Reviewers Checklist

  • Appropriate logging is done.
  • Appropriate error responses.
  • Handle every possible exception.
  • Make sure there is a docstring in the command functions. Hint: Lookout for
    botcmd and re_botcmd decorators.
  • See that 100% coverage is there.
  • See to it that mocking is not done where it is not necessary.

Copy link
Member

@jayvdb jayvdb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have two commits at https://github.com/coala/corobo/pull/606/commits

We dont allow merges in our repos. Ever. Dont do it.

Rebase.

@bhavya17037
Copy link
Contributor Author

@jayvdb
I will rebase in a while. Dont worry

@@ -15,9 +15,6 @@ class Coatils(BotPlugin):
Various coala related utilities, exposing the REST API, etc.
"""

def __init__(self, bot, name=None):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated change?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove this change from the file and do git commit --amend to modify the commit(do not use git commit)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But this change is already in corobo's "coatils.py" (master)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to fetch that change from upstream repo(master) by performing a rebase, refer http://api.coala.io/en/latest/Developers/Git_Basics.html#rebasing

testbot.assertCommand("!hey nM", 'Command "hey" / "hey nM" not found.')
testbot.assertCommand("!nevermindxyz", 'Command "nevermindxyz" not found.')
testbot.assertCommand(
"!hey nEverMINd", 'Command "hey" / "hey nEverMINd" not found.')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The code does not comply to PEP8.

Origin: PEP8Bear, Section: all.autopep8.

The issue can be fixed by applying the following patch:

--- a/tmp/tmp2chs3k8n/tests/nevermind_test.py
+++ b/tmp/tmp2chs3k8n/tests/nevermind_test.py
@@ -12,4 +12,4 @@
     testbot.assertCommand("!hey nM", 'Command "hey" / "hey nM" not found.')
     testbot.assertCommand("!nevermindxyz", 'Command "nevermindxyz" not found.')
     testbot.assertCommand(
-    "!hey nEverMINd", 'Command "hey" / "hey nEverMINd" not found.')
+        "!hey nEverMINd", 'Command "hey" / "hey nEverMINd" not found.')

testbot.assertCommand("!hey nM", 'Command "hey" / "hey nM" not found.')
testbot.assertCommand("!nevermindxyz", 'Command "nevermindxyz" not found.')
testbot.assertCommand(
"!hey nEverMINd", 'Command "hey" / "hey nEverMINd" not found.')

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E122 continuation line missing indentation or outdented

Origin: PycodestyleBear (E122), Section: all.autopep8.

Change regex pattern so that corobo apologizes only to commands
like "corobo nm" and "corobo nevermind". Add unit test cases
for the same.

Fixes coala#519
@TravisBuddy

This comment has been minimized.

@jayvdb
Copy link
Member

jayvdb commented Aug 7, 2018

ack 6608678

@jayvdb
Copy link
Member

jayvdb commented Aug 7, 2018

@gitmate-bot ff

@gitmate-bot
Copy link

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 gitmate-bot merged commit 6608678 into coala:master Aug 7, 2018
@gitmate-bot
Copy link

Automated fastforward with GitMate.io was successful! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants