-
Notifications
You must be signed in to change notification settings - Fork 28
Add unit tests for CLI parsing #243
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Andrew Helwer <[email protected]>
Signed-off-by: Andrew Helwer <[email protected]>
Signed-off-by: Andrew Helwer <[email protected]>
Signed-off-by: Andrew Helwer <[email protected]>
Signed-off-by: Andrew Helwer <[email protected]>
Signed-off-by: Andrew Helwer <[email protected]>
Signed-off-by: Andrew Helwer <[email protected]>
Signed-off-by: Andrew Helwer <[email protected]>
Signed-off-by: Andrew Helwer <[email protected]>
Signed-off-by: Andrew Helwer <[email protected]>
damiendoligez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good.
I don't know why you have many functions with type annotations for the arguments and results (at first glance I would say the compiler can infer most of them) but it doesn't really hurt.
Signed-off-by: Andrew Helwer <[email protected]>
|
The type annotations are a habit from rust/java I suppose. I like type annotations! Sometimes they also make error messages easier to understand. Yeah they make things verbose though. |
Intended to facilitate merge of PR #177 and form test base for any further command line modifications.
Tried to minimize changes to actual code; mostly required injecting
Format.formatterinstances to capture output and also trapping calls toexit.