Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make API depend only on jackson-annotations #1279

Merged
merged 8 commits into from Dec 3, 2019
Merged

Conversation

bsideup
Copy link
Member

@bsideup bsideup commented Nov 21, 2019

This change is Reviewable

@bsideup bsideup added this to the 3.2.0 milestone Nov 21, 2019
@codecov-io
Copy link

codecov-io commented Nov 22, 2019

Codecov Report

Merging #1279 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1279   +/-   ##
======================================
  Coverage      75%     75%           
======================================
  Files           1       1           
  Lines          16      16           
======================================
  Hits           12      12           
  Misses          3       3           
  Partials        1       1

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 066f268...d59d082. Read the comment docs.

@darinhoward
Copy link

Is it possible to use https://github.com/FasterXML/jackson-annotations as the dependency and keep this existing annotations ?

@bsideup
Copy link
Member Author

bsideup commented Dec 2, 2019

Why would you want to keep them?

The goal is to make API have zero dependencies.

@darinhoward
Copy link

I feel that the inclusion of a dependency of jackson-annotations (small api footprint) is a more simplistic solution than custom annotations and a custom JacksonAnnotationIntrospector. This would similar to the inclusion of slf4j-api.

@bsideup
Copy link
Member Author

bsideup commented Dec 3, 2019

Unlike Slf4j, Jackson does not provide an "api" artifact. It brings two dependencies (annotations and core) and makes it extremely hard to shade.

So, unless there is any issue with the introspection, I would like to proceed.

@darinhoward
Copy link

darinhoward commented Dec 3, 2019

I'm suggesting using this dependency - it has no dependency on core - it just contains the annotations. Now if that doesn't include all the annotations in use - then the custom annotations would be needed.

<dependency> <groupId>com.fasterxml.jackson.core</groupId> <artifactId>jackson-annotations</artifactId> <version>${jackson.version}</version> </dependency>

@bsideup
Copy link
Member Author

bsideup commented Dec 3, 2019

@darinhoward ok, although I think we can/should avoid Jackson in the public API, I guess a good first step will be to only use jackson-annotations, yes. We can introduce custom annotations later, but at least we can work with it. Thanks for the feedback.

Although I kept toPrimitive/fromPrimitive to avoid a dependency on Jackson's serialization abstractions, since otherwise we need another dependency

@bsideup bsideup changed the title Use custom annotations to avoid Jackson dependency Make API depend only on jackson-annotations Dec 3, 2019
@bsideup bsideup merged commit 255d7a6 into master Dec 3, 2019
@bsideup bsideup deleted the remove_jackson_from_api branch December 3, 2019 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants