Re: Now official: Sound Splitter 22.02, requesting review before submitting to community add-ons registry
Hi,toggle quoted messageShow quoted text
Yep, I'll take care of misspellings and tweak the help text for async generator.
Tony, what do you think about these comments as Sound Splitter ultimately came from your code?
From: firstname.lastname@example.org <email@example.com> On Behalf Of Lukasz Golonka via groups.io
Sent: Friday, January 28, 2022 6:09 AM
Subject: Re: [nvda-addons] Now official: Sound Splitter 22.02, requesting review before submitting to community add-ons registry
Basic review results:
* License and copyright: pass
* Documentation: pass
* Security: pass
* User experience: pass
* Code and code comments: pass with some nits:
- There are several misspells in the comments rright -> right, yilds -> yields
- Docstring for executeAsynchronously should be tweaked as it still references stuff from Tony's add-on e.g.GlobalPlugin.script_editJupyter
- All messages in the script and the script documentation itself don't have comments for translators.
- I haven't tested this explicitly but the add-on probably wouldn't work in source copes of NVDA since it relies on NVDA's process name. I'd suggest to rely on the PID of the process if possible and comparing it with the result of os.getpid.
- While I understand why you're monkeypatching nvwave (there is no better way for now) perhaps in the future it would be nice to add additional extension point into core of NVDA which would allow to execute arbitrary function after nvwave.WavePlayer.open executes.