Move msgpack value classes to another project msgpack-value by dmikurube · Pull Request #571 · msgpack/msgpack-java · GitHub
Skip to content

Move msgpack value classes to another project msgpack-value#571

Draft
dmikurube wants to merge 5 commits intomsgpack:mainfrom
dmikurube:split-msgpack-value-abstract-MessagePacker
Draft

Move msgpack value classes to another project msgpack-value#571
dmikurube wants to merge 5 commits intomsgpack:mainfrom
dmikurube:split-msgpack-value-abstract-MessagePacker

Conversation

@dmikurube
Copy link
Copy Markdown

@dmikurube dmikurube commented May 18, 2021

Some org.msgpack.value classes (typically org.msgpack.value.Value) are used in method signatures of other software's API/SPI, unfortunately.
https://dev.embulk.org/embulk-api/0.10.31/javadoc/org/embulk/spi/PageBuilder.html#setJson-org.embulk.spi.Column-org.msgpack.value.Value-

Against such cases, I was wondering if those value classes are separated in a different Maven artifact so that API/SPI includes only stable msgpack-value, and implementation is released from dependency hell...

Can I hear your thoughts?

I don't stick with the implementation shown in this PR, such as naming, class structures, and else. This PR is just as a basis for discussion. But such a separation is really helpful.

@dmikurube dmikurube mentioned this pull request May 18, 2021
8 tasks
@xerial
Copy link
Copy Markdown
Member

xerial commented May 18, 2021

@dmikurube
Copy link
Copy Markdown
Author

@xerial Thanks. Got it. I understand your concerns and plan.

Then, I think we'll make a choice of removing msgpack-java's classes from Embulk's API/SPI in a (very) long-term. Our plan would be:

The removal in Embulk API will be much much later to wait for plugins to catch up with the latest manner. But, I believe that making a clear declaration during v0.10 would be reasonable and making things smoother.

P.S. You may be interested in this: https://gist.github.com/dmikurube/c48ac6432cb4c2151ff9c75cd253bcd5

@xerial
Copy link
Copy Markdown
Member

xerial commented May 21, 2021

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.

2 participants