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

Support gather verb #8

Merged
merged 14 commits into from
Apr 14, 2015
Merged

Conversation

rudigiesler
Copy link

No description provided.

if numDigits < 1:
raise TwiMLParseError(
"Invalid value %r for numDigits parameter. Must be >=1"
% numDigits)

Choose a reason for hiding this comment

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

Maybe it's worthwhile factoring out a simple integer checking function?

@rudigiesler
Copy link
Author

Ready for review.

self.assertEqual(gather.nouns[0].name, 'Play')

def test_gather_from_xml_invalid_sub_verbs(self):
"""Shoudl raise an exception for invalid sub verbs"""

Choose a reason for hiding this comment

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

s/Shoudl/Should/ :)

@hodgestar
Copy link

Left one minor comment. Looks good though. There are a lot of TODOs, but I'd like to start trying things out with Rapid before we worry too much about being feature complete.

@rudigiesler
Copy link
Author

The TODOS are mostly things that I don't think are necessary for Rapid, but they're there for if something with Rapid doesn't work, we know where to put the feature that needs to be added. Also to keep a note of what we need to add in future if we want to make it feature complete.

@rudigiesler
Copy link
Author

Issues addressed.

@hodgestar
Copy link

👍 as soon as Travis is happy.

rudigiesler pushed a commit that referenced this pull request Apr 14, 2015
@rudigiesler rudigiesler merged commit f8d4db6 into develop Apr 14, 2015
@rudigiesler rudigiesler deleted the feature/issue-8-support-gather-verb branch April 14, 2015 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants