Add description to optional parameters#32
Add description to optional parameters#32victormarante wants to merge 2 commits intoModernRonin:mainfrom
Conversation
|
got a test failing, see the CI build |
|
also, please write tests for the new behaviors. |
|
I have added some tests. As for the failing test, How do we wish to handle this? |
|
it seems there are still tests failing? |
|
the one you mention, |
|
(also sorry for my being not very much here lately, but I have both stress at work and one of my two cats is in the final stages of renal dysfunction, I just wrote an email to the vet asking whether she thinks it's time to put him to sleep) |
ModernRonin
left a comment
ModernRonin
left a comment
There was a problem hiding this comment.
I reviewed this now, please check the comments. And there are more tests that fail than just the one you mentioned (and for that one I wrote a reply, see the PR discussion)
| else | ||
| { | ||
| result.RightLines.Add($"Default: {opt.Default}"); | ||
| } |
There was a problem hiding this comment.
this can be simplified to result.RightLines.Add(opt.Default is null ? opt.Description : $"Default: {opt.Default}"
| { | ||
| throw new InvalidOperationException( | ||
| $"Default are only settable for optional parameters - maybe you forgot a call to {nameof(MakeOptional)}?"); | ||
| } |
There was a problem hiding this comment.
would be good to have a test for the throw branch, too
| public void Description_Should_Not_Have_Error_When_Description_Empty_For_Value_Type() => | ||
| new OptionalParameterValidator() | ||
| .TestValidate(new OptionalParameter { Type = typeof(int) }) | ||
| .ShouldHaveValidationErrorFor(p => p.Description); |
There was a problem hiding this comment.
shouldn't it be .ShouldNotHaveValidationErrors() or some such in this test-case?
Hi. I will try to look at this as soon as possible. |
2 participants
Add description for optional parameters. If the property type has a default value of
null, throw an error saying description is required.