Closed graemegowans closed 4 years ago
Thanks @graemegowans! Will try to do a review for you next week.
Hi @graemegowans,
This is great work. The unit tests in particular are about as thorough as I've ever seen.
The changes I want to suggest are mostly cosmetic. The functionality is great and I definitely wouldn't want anything drastic. This is my penultimate week at PHS and I'm determined to do another release of the package next week. So, in the interest of time, I've made the changes myself on your branch, rather than leaving comments and asking you to. This is terrible practice and not something I'd do normally, but I'd like to avoid a mad rush to get everything finished next week. These aren't changes you'd have been incapable of making - sorry if I've deprived you of anything.
The main changes are:
chi_check()
and left them as standalone, non-exported functions within the script. I think it'll make it easier to debug if something goes wrong.chi_pad()
.chi_check()
instead of it returning an NA
for valid CHIs. NA
s are just a bit of a pain to deal with.I hope this is alright. Please have a look through the scripts and let me know if there's anything you're not happy with, any typos you spot, etc.
Cheers,
Jack
Hey @jackhannah95, thanks for reviewing and the nice words! Glad it was ok. Those changes all look good, thanks for making them. Please let me know if you need me to do anything else. Good luck with the release and your next role! Cheers.
Any problems let me know! Thanks.