Skip to content
This repository was archived by the owner on Apr 27, 2026. It is now read-only.

Make value kernel extensible#2939

Merged
colombod merged 2 commits into
dotnet:mainfrom
colombod:make_value_kenel_extensible
Apr 25, 2023
Merged

Make value kernel extensible#2939
colombod merged 2 commits into
dotnet:mainfrom
colombod:make_value_kenel_extensible

Conversation

@colombod

Copy link
Copy Markdown
Member

No description provided.

{
mimeType ??= (options.MimeType ?? PlainTextFormatter.MimeType);

var shouldDisplayValue = options.MimeType is { } displayMimeType;

Copy link
Copy Markdown
Contributor

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.

context.DisplayAs(value, mimeType);
}

return Task.CompletedTask;

Copy link
Copy Markdown
Contributor

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?

message: "The --from-file option cannot be used in combination with a content submission.");
}

Copy link
Copy Markdown
Contributor

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 mimeType,
bool shouldDisplayValue,
KernelInvocationContext context)

Copy link
Copy Markdown
Contributor

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...

@shyamnamboodiripad shyamnamboodiripad left a comment

Copy link
Copy Markdown
Contributor

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!

@colombod
colombod merged commit e1620e3 into dotnet:main Apr 25, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants