Skip to content

Conversation

vidartf
Copy link
Contributor

@vidartf vidartf commented Aug 7, 2017

Adds an argument buffers to services.kernels.comm.Comm.open()/close(). It is passed through to the send_shell_message similarly to what is done in Comm.send().

Fixes #2730.

@stonebig
Copy link

stonebig commented Aug 7, 2017

#2730 ?

@takluyver takluyver added this to the 5.1 milestone Aug 7, 2017
@takluyver
Copy link
Member

Thanks. Should CommManager.prototype.new_comm also gain a buffers parameter and pass it through to comm.open()?

@takluyver
Copy link
Member

@minrk @gnestor I've marked this as 5.1 since it's a fairly simple change that makes sense to me, and seems unlikely to have adverse effects.

@vidartf
Copy link
Contributor Author

vidartf commented Aug 7, 2017

@takluyver I added the buffers argument to new_comm as well. I added it at the end to retain compatibility with old signatures, even if the grouping of arguments then gets a little different.

@takluyver
Copy link
Member

Thanks, I think that's the right thing to do.

@gnestor gnestor merged commit 7dc9439 into jupyter:master Aug 7, 2017
@gnestor
Copy link
Contributor

gnestor commented Aug 7, 2017

Thanks @vidartf 1

@vidartf vidartf deleted the comm-buffers branch August 7, 2017 21:48
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 5, 2021
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.

JS services' Comm.open/close should take buffers

4 participants