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

Add support for odk:recordaudio #519

Merged
merged 8 commits into from
Feb 17, 2021
Merged

Conversation

seadowg
Copy link
Contributor

@seadowg seadowg commented Feb 17, 2021

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:

  • included test cases for core behavior and edge cases in tests_v1
  • run nosetests and verified all tests pass
  • run black pyxform to format code
  • verified that any code or assets from external sources are properly credited in comments

@seadowg seadowg marked this pull request as ready for review February 17, 2021 17:18
@lognaturel lognaturel changed the title Add support for odk:recordaction Add support for odk:recordaudio Feb 17, 2021
Copy link
Contributor

@lognaturel lognaturel left a 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!

pyxform/question_type_dictionary.py Show resolved Hide resolved
pyxform/tests_v1/test_background_audio.py Show resolved Hide resolved
pyxform/tests_v1/test_background_audio.py Outdated Show resolved Hide resolved
@codecov-io
Copy link

Codecov Report

Merging #519 (80fc54e) into master (7e86931) will increase coverage by 0.06%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            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              
Impacted Files Coverage Δ
pyxform/question_type_dictionary.py 57.14% <ø> (ø)
pyxform/constants.py 100.00% <100.00%> (ø)
pyxform/xls2json.py 78.65% <100.00%> (+0.32%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7e86931...6a7b49d. Read the comment docs.

@seadowg seadowg requested a review from lognaturel February 17, 2021 18:08
@lognaturel lognaturel merged commit 6886234 into XLSForm:master Feb 17, 2021
@seadowg seadowg deleted the recordaction branch February 17, 2021 19:33
@lognaturel
Copy link
Contributor

@MartijnR it occurs to me you might like (and it might be a good idea) to do a sanity check on this.

@MartijnR
Copy link
Contributor

Thanks. I had a quick look at the tests. All looks good to me!

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.

Add background-audio type
4 participants