ARROW-3630: [Plasma][GLib] Add GLib bindings of Plasma#2848
ARROW-3630: [Plasma][GLib] Add GLib bindings of Plasma#2848shiro615 wants to merge 13 commits intoapache:masterfrom
Conversation
shiro615
force-pushed
the
glib-support-plasma
branch
from
4af3bed to
12f0389
Compare
shiro615
changed the title
kou
left a comment
kou
left a comment
There was a problem hiding this comment.
Thanks!
This is a good start point!
c_glib/plasma-glib/plasma-client.h
Outdated
| G_BEGIN_DECLS | ||
|
|
||
| #define GPLASMA_TYPE_PLASMA_CLIENT (gplasma_plasma_client_get_type()) | ||
| G_DECLARE_DERIVABLE_TYPE(GPlasmaPlasmaClient, |
There was a problem hiding this comment.
Can you rename to GPlasmaClient? PlasmaPlasma is redundant.
c_glib/plasma-glib/plasma-client.cpp
Outdated
| gplasma_plasma_client_new(void) | ||
| { | ||
| auto plasma_plasma_client = std::make_shared<plasma::PlasmaClient>(); | ||
| return gplasma_plasma_client_new_raw(&plasma_plasma_client); |
There was a problem hiding this comment.
Can you change to gplasma_client_new(const gchar *store_socket_name, GError **error)?
GPlasmaClient *
gplasma_client_new(const gchar *store_socket_name, GError **error)
{
auto plasma_client = std::make_shared<plasma::PlasmaClient>();
auto status = client->Connect(store_socket_name, "");
// ...
}We will add more constructors for manager_socket_name, release_delay and num_retries in PlasmaClient::Connect() later.
There was a problem hiding this comment.
OK! I'll change to gplasma_client_new(const gchar *store_socket_name, GError **error).
| end | ||
|
|
||
| def test_new | ||
| assert_nothing_raised do |
There was a problem hiding this comment.
We can test connecting to store server by running plasma_store_server.
We can get plasma_store_server path from plasma.pc: pkg-config --variable=executable plasma
There was a problem hiding this comment.
I’ve added test cases.
Please confirm my understanding.
There was a problem hiding this comment.
I've confirmed. You understand correctly.
I've pushed some minor improvements such as adding missing waitpid to avoid zombi process and moving to helper/.
Codecov Report
@@ Coverage Diff @@
## master #2848 +/- ##
==========================================
+ Coverage 87.55% 87.56% +0.01%
==========================================
Files 411 412 +1
Lines 63818 64036 +218
==========================================
+ Hits 55874 56074 +200
- Misses 7870 7888 +18
Partials 74 74
Continue to review full report at Codecov.
|
|
Thank you for review. I'll try to address them. |
shiro615
force-pushed
the
glib-support-plasma
branch
from
91a9927 to
5c7a850
Compare
shiro615
changed the title
|
Merged. Thanks! |
3 participants
It's GLib bindings of Plasma.