ARROW-4005: [Plasma] [GLib] Add gplasma_client_disconnect()#3163
ARROW-4005: [Plasma] [GLib] Add gplasma_client_disconnect()#3163shiro615 wants to merge 5 commits intoapache:masterfrom
Conversation
kou
left a comment
kou
left a comment
There was a problem hiding this comment.
Thanks!
Can you check my comment?
c_glib/plasma-glib/client.cpp
Outdated
| GError **error) | ||
| { | ||
| auto plasma_client = gplasma_client_get_raw(client); | ||
| auto status = plasma_client->Disconnect(); |
There was a problem hiding this comment.
We need to keep "disconnected" information.
If users don't call gplasma_client_disconnect() explicitly, we call Disconnect() on fainalize().
if (garrow_error_check(...)) {
priv->disconnected = true;
return TRUE;
} else {
return FALSE;
}There was a problem hiding this comment.
I’ll keep "disconnected” information. If priv->disconnected is true, I will not call Disconnected on finalize().
| auto priv = GPLASMA_CLIENT_GET_PRIVATE(client); | ||
| auto status = priv->client->Disconnect(); | ||
| if (garrow_error_check(error, status, "[plasma][client][disconnect]")) { | ||
| priv->disconnected = true; |
There was a problem hiding this comment.
We should use bool instead of gboolean when we use true.
There was a problem hiding this comment.
I see. I'll use bool.
| end | ||
|
|
||
| test("#disconnect") do | ||
| require_gi(1, 42, 0) |
There was a problem hiding this comment.
Can we remove this by using options?
There was a problem hiding this comment.
Yes. I've removed require_gi.
kou
left a comment
kou
left a comment
There was a problem hiding this comment.
+1
I'll merge this when CI is green.
2 participants
No description provided.