Closed thorhj closed 4 years ago
Thanks! I will take a look.
@Tyrrrz I have added a lot of commits to fix the issues you mentioned. There are still missing tests, but I don't think I can get to it the following week. You are welcome to implement tests if you wish, otherwise it will take some time :)
Sounds good and don't worry. I'm currently overloaded with issues myself (both real life and other projects) so I can't devote my full focus to CliFx yet :)
Just so I'll remember, here is the stuff I need to write tests on:
Command resolution
Help text
Argument validation
Input validation (command initialization)
NOTE: Consider moving validation to separate class to allow easy unit testing without creating numerous test commands.
Added some conflicts for you to make it more fun.
@Tyrrrz I am picking this up again. Hope to have it done in a few days.
@Tyrrrz Happy new years! I think all the work in this PR is done now -- what better way to start the new year than to code review? ;)
@thorhj Sounds like the best New Years gift to me :) I'll look at it soon. Happy holidays to you too!
@thorhj how is it going? :) Any chance you will have time to fix the few issues so I can merge it next week?
@Tyrrrz It should be fixed now!
Thanks! 🎉
This is for issue #26 with the following shortcomings:
Tests are missing Will implement if the solution is approved.
Closes #26