-
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
js share by reference formatted values #3032
Conversation
if is byref OR is not formatted value simplify condition proxykernel should not remove targetkernel name revert proxy kernel changes check is proxy remoteuri clean test
@@ -494,6 +494,43 @@ public async Task value_option_is_required() | |||
.Which.Message.Should().Be("Option '--value' is required."); | |||
} | |||
|
|||
[Fact] | |||
public async Task ProxyKernels_can_share_values() |
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.
This test name should be more specific.
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.
this ProxyKernels_sharing_values_receives_them_as_deserialized_values
|
||
ValueProduced valueProduced; | ||
if ( | ||
sourceKernel?.KernelInfo.IsProxy == false |
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 don't believe KernelInfo
can ever be null.
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.
if the kernel is not found. that is the case when using @input:x
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.
Ah right.
@@ -383,6 +361,8 @@ ValueOptionResult ParseValueOption(ArgumentResult argResult) | |||
isByref = false; | |||
} | |||
|
|||
var valueSourceKernel = destinationKernel.RootKernel.FindKernelByName(sourceValueName); |
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.
No longer needed?
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.
ops
Variable shared from js are now correctly parsed