-
Notifications
You must be signed in to change notification settings - Fork 389
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
Make value kernel extensible #2939
Conversation
{ | ||
mimeType ??= (options.MimeType ?? PlainTextFormatter.MimeType); | ||
|
||
var shouldDisplayValue = options.MimeType is { } displayMimeType; |
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.
nit: The displayMimeType
variable is unused and can be dropped.
} | ||
|
||
return Task.CompletedTask; |
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.
Can the method be made sync or is this async because we think we may need it down the line?
@@ -143,7 +143,7 @@ Task IKernelCommandHandler<SendValue>.HandleAsync(SendValue command, KernelInvoc | |||
context.Fail(command, | |||
message: "The --from-file option cannot be used in combination with a content submission."); | |||
} | |||
|
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.
minor: Remove blank line.
string value, | ||
string mimeType, | ||
bool shouldDisplayValue, | ||
KernelInvocationContext context) | ||
|
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.
minor: Consider removing blank line...
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.
Change looks good - however, I am not sure why this is needed. Could you please clarify? Thanks!
No description provided.