search

mikaelnousiainen / vexabc

ABC notation parser and renderer for VexFlow
Mozilla Public License 2.0
23 stars 4 forks source link

Updated duration generator to handle any duration value #2

Closed cgamache closed 10 years ago

cgamache commented 11 years ago

So, value is a numeric type with my edits. Not sure if that matters a whole lot. I'm still getting a feel for the library. If it does, it's an easy coercion to a string type.

incompleteopus commented 10 years ago

Thanks for the patch, would it be possible for you to write a couple of tests for the function before I merge it? I'm resuming VexABC development and any kind of help with the project is greatly appreciated!

cgamache commented 10 years ago

No problem, glad to do it!

Looking at the tests in the codebase already it looks like they require a visual inspect. Are you looking for me to add a test where the output is tested visually, or a test with some assertions on the output data?

On Wed, Oct 30, 2013 at 1:47 PM, incompleteopus notifications@github.comwrote:

Thanks for the patch, would it be possible for you to write a couple of tests for the function before I merge it? I'm resuming VexABC development and any kind of help with the project is greatly appreciated!

— Reply to this email directly or view it on GitHubhttps://github.com/incompleteopus/vexabc/pull/2#issuecomment-27417867 .

incompleteopus commented 10 years ago

Great! Assertions on output data are probably enough in this case, as what is being tested is an algorithm. Not all tests are required to render notation and note durations are tested in separate tests.

cgamache commented 10 years ago

Good plan! Changes made. The unit test behaves as expected now. Thank you for the clarification on the multi-measure rests.

incompleteopus commented 10 years ago

Thanks, the patch is good now. Merging.