ARROW-2690: [Plasma] Use uniform function names in public APIs in Plasma. Add namespace around Flatbuffers#2242
ARROW-2690: [Plasma] Use uniform function names in public APIs in Plasma. Add namespace around Flatbuffers#2242wesm wants to merge 3 commits intoapache:masterfrom
Conversation
cpp/src/plasma/protocol.cc
Outdated
There was a problem hiding this comment.
When I was working with the IPC code, it was hard to distinguish between flabuffers and flatbuf.
Later You might consider to use a namespace which implies that it belongs to arrow (something like arrowfbs or whatever instead of flatbuf)
There was a problem hiding this comment.
A fair point. I can make that change
cpp/src/plasma/protocol.cc
Outdated
cpp/src/plasma/protocol.cc
Outdated
Change-Id: Iace9fd61a7d77922d2481ade7479752a7d509e45
Change-Id: Ib9c58487ab198b5f86d1176b8a66808a72fc571e
|
@pcmoritz @robertnishihara could you have a look at this? It's largely cosmetic but the public API for Plasma is impacted. Better to clean up sooner rather than later How important is it to have details of the client wire protocol in the public API (do you need to get access to those headers in Ray)? Should we try to fix that? |
Codecov Report
@@ Coverage Diff @@
## master #2242 +/- ##
==========================================
+ Coverage 84.26% 86.65% +2.38%
==========================================
Files 290 234 -56
Lines 44299 41558 -2741
==========================================
- Hits 37330 36012 -1318
+ Misses 6942 5546 -1396
+ Partials 27 0 -27
Continue to review full report at Codecov.
|
|
This looks good to me, thanks for the cleanups! |
|
@wesm The wire protocol isn't exposed through the public API, is it? |
|
@robertnishihara It is exposed. Do you use the Flatbuffers APIs directly anywhere in Ray? Otherwise we should hide these symbols in the Plasma public headers and stop installing the generated Flatbuffers bindings |
|
@wesm: At the moment we do but can get rid it! I can spend some time cleaning up the plasma code in this regard. Did you already create a JIRA, I couldn't find one? |
5 participants
I made a pass over Plasma to make the function names more uniform in the style; let me know your comments. It was a little bit painful.
I also wanted to be more explicit about what parts of the Flatbuffers protocol are leaking into the public API. Still
flatbuffers/flatbuffers.his leaking, but this is a step in containing things a bit.