Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
MockPromptForString + doctor to work with ValidatePathError. #325
MockPromptForString + doctor to work with ValidatePathError. #325
Changes from 1 commit
df1c828
556dacd
fac428d
acba2ea
b3fc03c
1b4ffdf
2c18f9d
745c35c
5897c8e
011f86d
6e1020d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
I just realised this isn't actually "for strings" anymore - it can just be called MockPrompt now.
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.
But the messages are always strings
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.
I had thought that the "ForStrings" was about the return type of the methods - for example that promptInput would always return a string (as opposed to a Color or something). The version we've got now works if it's used in a place that returns a Color.
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.
The "forStrings" was for the input we accept, and as for now it's only strings
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.
I had the same impression about
T
always being astring
, which affects thevalidateFunction
input and thePromise<T>
output. ButT
can now have any values and we could dropForStrings
. The fact thataddMessages()
is always a string seems to be more of an implementation detail?