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

Create TwilML parser #1

Merged
merged 20 commits into from
Mar 20, 2015
Merged

Conversation

rudigiesler
Copy link

No description provided.

- "pip install coveralls"
- "python setup.py install"
script:
- "coverage run --source=vumi_twilio_api `which py.test` vumi_twilio_api"

Choose a reason for hiding this comment

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

Don't know why I can only see Simon dH's comment on the indenting in email and not GitHub.

@hodgestar
Copy link

👍 on landing and iterating from here. A few general comments:

  • We should perhaps use Twisted's trial.unittest instead of unittest since this is going to be Twisted based.
  • It seems like there should be a cleaner way to assert on the exceptions (perhaps a combination of Twisted's f = self.assertRaises(...) or just a helper function).

from setuptools import setup, find_packages

setup(
name="vumi_twilio_api",

Choose a reason for hiding this comment

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

I'm wondering if vxtwilio might be a better name?

Choose a reason for hiding this comment

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

Also wondering whether using Twilio in the name might cause trademark issues? If so, maybe vxtwinio? :)

@rudigiesler
Copy link
Author

@hodgestar I've addressed your comments. Ready for another review

@hodgestar
Copy link

👍 !

rudigiesler pushed a commit that referenced this pull request Mar 20, 2015
@rudigiesler rudigiesler merged commit 25e3c1e into develop Mar 20, 2015
@rudigiesler rudigiesler deleted the feature/issue-1-create-twilml-parser branch March 20, 2015 08:26
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.

None yet

2 participants