Skip to content
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.

Sign up for GitHub

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

Jump to bottom

fix #2095: remove SubmitCode.SubmissionType #3174

Merged
merged 1 commit into from
Sep 14, 2023
Merged

fix #2095: remove SubmitCode.SubmissionType #3174

merged 1 commit into from
Sep 14, 2023

Conversation

jonsequitur
Copy link
Contributor

@jonsequitur jonsequitur commented Sep 14, 2023 •
edited
Loading

Fixes #2095.

shyamnamboodiripad
shyamnamboodiripad reviewed Sep 14, 2023
View reviewed changes
src/Microsoft.DotNet.Interactive.Jupyter/IsCompleteRequestHandler.cs
break;
case IncompleteCodeSubmissionReceived _:
Reply(false, context.JupyterRequestMessageEnvelope, context.JupyterMessageSender);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Were the removed code paths pretty much unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're unused after this code change.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not very familiar with the code, but the current class is called IsCompleteRequestHandler and I was a bit confused since it seemed like it was meant to check whether the complete submission has been received while now it checks whether or not the submission contains errors. Do we need to rename this for clarity?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Checking for errors is an implementation detail. This handler's purpose is the same as before, which is to inform the Jupyter console as to whether a code submission is "complete". This isn't a common use case for us in any event.

@jonsequitur jonsequitur enabled auto-merge (squash) September 14, 2023 01:44
@jonsequitur jonsequitur merged commit 35d05d5 into dotnet:main Sep 14, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shyamnamboodiripad shyamnamboodiripad shyamnamboodiripad approved these changes

@colombod colombod Awaiting requested review from colombod

Assignees
No one assigned
Labels
Area-API
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

remove SubmitCode.SubmissionType
3 participants
@jonsequitur @shyamnamboodiripad @colombod