-
Notifications
You must be signed in to change notification settings - Fork 137
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
Add support for odk:recordaudio #519
Conversation
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.
Looks good, just a few things!
22225e3
to
6a7b49d
Compare
Codecov Report
@@ Coverage Diff @@
## master #519 +/- ##
==========================================
+ Coverage 83.86% 83.92% +0.06%
==========================================
Files 25 25
Lines 3699 3713 +14
Branches 862 865 +3
==========================================
+ Hits 3102 3116 +14
Misses 452 452
Partials 145 145
Continue to review full report at Codecov.
|
@MartijnR it occurs to me you might like (and it might be a good idea) to do a sanity check on this. |
Thanks. I had a quick look at the tests. All looks good to me! |
Closes #517
Why is this the best possible solution? Were any other approaches considered?
I'm not that familiar with the code base so would not be surprised if there's a better way of parsing the parameters than what I ended up with. Mostly just put this in following how other actions/questions were handled.
What are the regression risks?
Shouldn't be any really as most code refers to a new question type.
Before submitting this PR, please make sure you have:
tests_v1
nosetests
and verified all tests passblack pyxform
to format code