Skip to content

Conversation

@icexelloss
Copy link
Contributor

Move ArrowMagic from vector.ipc to vector.ipc.message package.

@icexelloss
Copy link
Contributor Author

cc @BryanCutler

@icexelloss icexelloss changed the title ARROW-1047: [Java] [FollowUp] Add Generic Reader Interface for Stream Format ARROW-1047: [Java] [FollowUp] Move ArrowMagic to ipc.message package Nov 22, 2017
@wesm wesm force-pushed the ARROW-1047-followup branch from d53ff7f to b56defb Compare November 23, 2017 02:05
Copy link
Member

@wesm wesm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, though ArrowMagic is only used in the file reader/writer

@BryanCutler
Copy link
Member

I was thinking of ArrowMagic more being part of the file protocol than a message, but it's fine with me if you prefer to move it there.

@wesm
Copy link
Member

wesm commented Nov 23, 2017

I suggest we do not merge this because the magic number is only used in the file implementation, if that's OK

@icexelloss
Copy link
Contributor Author

Yes that's good point. I will change the PR to make it ArrowMagic non public class.

@icexelloss icexelloss changed the title ARROW-1047: [Java] [FollowUp] Move ArrowMagic to ipc.message package ARROW-1047: [Java] [FollowUp] Change ArrowMagic to be non-public class Nov 23, 2017
@wesm
Copy link
Member

wesm commented Nov 23, 2017

+1

@wesm wesm closed this in 6ec4f34 Nov 23, 2017
@wesm wesm deleted the ARROW-1047-followup branch November 23, 2017 18:26
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
Move ArrowMagic from vector.ipc to vector.ipc.message package.

Author: Li Jin <ice.xelloss@gmail.com>

Closes apache#1349 from icexelloss/ARROW-1047-followup and squashes the following commits:

0b5df95 [Li Jin] Change ArrowMagic to be non public class
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants