Skip to content

V07 add jackson dataformat#146

Merged
oza merged 79 commits intov07-developfrom
v07-add-jackson-dataformat
Nov 13, 2014
Merged

V07 add jackson dataformat#146
oza merged 79 commits intov07-developfrom
v07-add-jackson-dataformat

Conversation

@komamitsu
Copy link
Copy Markdown
Member

@komamitsu komamitsu commented Nov 9, 2014

See #145

komamitsu added 30 commits June 14, 2014 22:51
@komamitsu
Initial commit
5b27ce5
@komamitsu
Prototype MessagePackGenerator
579e43c
@komamitsu
Support nested objects
99688c6
@komamitsu
MessagePackGenerator supports Array
f97ebd8
@komamitsu
Add tests
1d46d19
@komamitsu
Implementing MessagePackParser and Codec
6985965
@komamitsu
Passed an unit test of MessagePackParser
c2679d3
@komamitsu
Fix package name and improve MessagePackParser
b3ba082
@komamitsu
Fix MessagePackParser's bug on a nested object
51ade22
@komamitsu
Minor refactoring
8d0b5f7
@komamitsu
MessagePackParser supports Array
7ea00f2
@komamitsu
Fix the bug on nested Object/Array
7acda0e
@komamitsu
Add testJsonProperty()
d6508cb
@komamitsu
Refactor tests
634de6d
@komamitsu
Support binary value
a85aef2
@komamitsu
MessagePackParser supports the serialization of binary data
14ecfde
@komamitsu
Add some tests for POJO
e65a213
@komamitsu
Rename test classes
0f6ad5f
@komamitsu
Replace MessagePackParser.currentXxxxx with valueHolder
5058a73
@komamitsu
Minor refactoring
8688a52
@komamitsu
[maven-release-plugin] prepare release jackson-dataformat-msgpack-0.0.1
944837c
@komamitsu
[maven-release-plugin] prepare for next development iteration
3bb9b49
@komamitsu
Add benchmark
0616475
@komamitsu
MessagePackParser(byte[]) calls MessageUnpacker(byte[])
74886d0
@komamitsu
Add some benchmark tests
333c0da
@komamitsu
Fix wrong packing in MessagePackParser.nextToken()
d4b8e6c
@komamitsu
Refactor unit tests
7a7aa1a
@komamitsu
Add maven-findbugs-plugin
e6f2cc9
@komamitsu
Add findbugs report and refactoring
1fecae8
@komamitsu
Use MesssagePacker/Unpacker.reset()
bd17525
xerial
xerial reviewed Nov 9, 2014
View reviewed changes
Comment thread project/Build.scala
Copy link
Copy Markdown
Member

@xerial xerial Nov 9, 2014

Choose a reason for hiding this comment

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

You can add name property here to specify jar name (e.g., jackson-databind-msgpack)

Copy link
Copy Markdown
Member Author

@komamitsu komamitsu Nov 10, 2014

Choose a reason for hiding this comment

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

👍

@xerial xerial added this to the v07 milestone Nov 9, 2014
xerial
xerial reviewed Nov 9, 2014
View reviewed changes
Comment thread msgpack-jackson/src/main/java/org/msgpack/jackson/dataformat/msgpack/MessagePackFactory.java Outdated
Copy link
Copy Markdown
Member

@xerial xerial Nov 9, 2014

Choose a reason for hiding this comment

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

MessagePackFactory is already used in msgpack-core. Can you rename it?

Copy link
Copy Markdown
Member Author

@komamitsu komamitsu Nov 10, 2014

Choose a reason for hiding this comment

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

Hmm. Jackson-dataformat-xxxx uses a naming convention (Data format name)Factory such as SmileFactory, CsvFactory, YAMLFactory, XmlFactory and AvroFactory. So I can't come up with a better name than MessagePackFactory. Also, I think having the same class names isn't an import issue because ordinary developers don't need to consider it other than developers of jackson-dataformat-msgpack (off course, we can use the correct class with each package name).

Copy link
Copy Markdown
Member

@xerial xerial Nov 11, 2014

Choose a reason for hiding this comment

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

But in the msgpack-jackson/README.md, you mentioned

only you need to do is to instantiate MessagePackFactory
https://github.com/msgpack/msgpack-java/pull/146/files#diff-c1644750de4c3267355de3f00a0f6f05R20

It's already ambiguous.

Copy link
Copy Markdown
Member

@xerial xerial Nov 11, 2014

Choose a reason for hiding this comment

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

Removed core.MessagePackerFactory, and packer/unpacker can be instantiated from MessagePack class, so it's okay to leave jackson/MessagePackFactory class name unchanged.

Copy link
Copy Markdown
Member Author

@komamitsu komamitsu Nov 11, 2014

Choose a reason for hiding this comment

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

All right. Thanks.

xerial
xerial reviewed Nov 11, 2014
View reviewed changes
Comment thread README.md Outdated
Copy link
Copy Markdown
Member

@xerial xerial Nov 11, 2014

Choose a reason for hiding this comment

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

Please explain what is jackson-databind. For example,

You can use jackson-databind to serialize/deserialize Java objects in MessagePack format. For details, see msgpack-jackson/README.md

@xerial
Copy link
Copy Markdown
Member

xerial commented Nov 11, 2014

package name org.msgpack.jackson.dataformat.msgpack is a little bit long. Is it possible to shorten it to 'org.msgpack.jackson'?

@xerial
Copy link
Copy Markdown
Member

xerial commented Nov 11, 2014

To avoid the name conflict of MessagePackFactory , I migrated the functionality of core.MessagePackFactory to MessagePack class.

@komamitsu
Copy link
Copy Markdown
Member Author

komamitsu commented Nov 11, 2014

package name

Good point. I'll change it.

@komamitsu
Copy link
Copy Markdown
Member Author

komamitsu commented Nov 12, 2014

@xerial I made the package name shorter.

@xerial
Copy link
Copy Markdown
Member

xerial commented Nov 12, 2014

@komamitsu
Is dataformat package necessary? If we do not add any class under org.msgpack.jackson package, we can move the classes in org.msgpack.jackson.dataformat to org.msgpack.jackson.

If there is a possibility to add some org.msgpack.jackson.XXX class in future, using org.msgpack.jackson.dataformat is ok.

@komamitsu
Copy link
Copy Markdown
Member Author

komamitsu commented Nov 13, 2014

@xerial I think it's possible we extend jackson core library as org.msgpack.jackson.core.* in the future. So we'd better to keep the package name org.msgpack.jackson.dataformat.

xerial and others added 3 commits November 13, 2014 15:53
@oza
Copy link
Copy Markdown
Member

oza commented Nov 13, 2014

Looks good to me. Merging.

oza added a commit that referenced this pull request Nov 13, 2014
@oza
Merge pull request #146 from msgpack/v07-add-jackson-dataformat
cb918ab 
V07 add jackson dataformat
@oza oza merged commit cb918ab into v07-develop Nov 13, 2014
@xerial xerial deleted the v07-add-jackson-dataformat branch November 13, 2014 08:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

v07

Development

Successfully merging this pull request may close these issues.

3 participants

@komamitsu @xerial @oza