Bs1770gainsupport #1343
Bs1770gainsupport #1343
Conversation
|
Thanks for updating this! For posterity: this is a continuation of the work in #1340. For future reference, @jmwatte, it's possible to add commits to an existing pull request. (In fact, you can see that happened automatically at the bottom of #1340.) Just push to the same branch the active PR is on. |
| @@ -32,12 +33,14 @@ | |||
| # Utilities. | |||
|
|
|||
| class ReplayGainError(Exception): | |||
|
|
|||
There was a problem hiding this comment.
This extra whitespace is not needed. Can you please delete it again?
|
Overall, this is looking good. Thank you for the contribution; this backend looks like the most versatile one yet. I have just a few suggestions above. We can add this without tests for now, but this might be a great learning opportunity! It would be awesome to have some code that demonstrates that this works. Can I convince you to look into the existing RG backend tests? They should be easily adaptable to the new backend. We can do this as the next project after merging the core functionality. |
|
I'll see if I can figure out the test. One question... I have made the suggested changes and pushed them to my git-fork. Does it then come to here automatically or do I have to do something else... You've got to love a noob... |
|
Yes! It's that easy. Just push new commits to the same branch and they will appear here. |
| call([cmd, self.method]) | ||
| self.command = cmd | ||
| except OSError: | ||
| pass |
There was a problem hiding this comment.
I think that
except OSError:
raise FatalReplayGainError('...')
would be terser & more elegant
|
Thank you! And thanks for your astute comments, @brunal. All merged. I look forward to seeing how the tests go—please let me know if I can help with those. |
|
I just pushed my first try at the tests... But I have no idea if that is what you mean... Just shooting in the dark. I did install all the dependencies and ran the tests.... Deleted my bs1770gain from my path... The tests were skipped... And passed when I put it back... So.... Let me know... —Reply to this email directly or view it on GitHub. |
I hope I did it beter with this one... Still learning my way around this git and github thing. I haven't been able to do that test-thing because frankly I've never done it before...
Already thxs for the previous remarks.