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

Fix #81 (update snakeyaml version to 1.21) #82

Closed
wants to merge 1 commit into from

Conversation

yborovikov
Copy link

This is a (minimalistic) fix for the backwards-incompatible refactoring in snakeyaml 1.20.

Note: if backwards compatibility with pre-1.20 snakeyaml is strongly desired we could introduce a wrapper with runtime detection of *Event constructor signatures. @cowtowncoder?

@cowtowncoder
Copy link
Member

I think ideally I would avoid dynamic discovery/linkage if possible. But it all comes down to how common there are to have transitive dependencies. Also, unfortunately use of shading of snakeyaml is apparently problematic for OSGi (for reasons I don't even fully understand myself), solution which could otherwise remove problem altogether.

I think that one possible path here is to:

  1. Keep Jackson 2.x at earlier version (pre-performance issue)
  2. Make Jackson 3.0 require 1.20 or later (wrt compatibility)

jackson 3 is still quite a ways off (first release perhaps by end of 2018) so to me that seems plausible path.

@yborovikov
Copy link
Author

yborovikov commented Apr 25, 2018

Ok, in this case, we likely should:

  1. Downgrade snakeyaml version back to 1.17 (re (yaml) Possible performance regression in snakeyaml 1.18 #67, especially if there is a reproducible benchmark/test and the degradation is measurable/significant) in Jackson 2.9.6?
  2. Apply this PR to Jackson 3.0 master (and not the 2.9 branch).

Is my understanding correct?

@cowtowncoder
Copy link
Member

@yborovikov Yes.

@asomov
Copy link
Contributor

asomov commented May 11, 2018

It should be better to use SnakeYAML engine in Jackson 3.0

@yborovikov
Copy link
Author

Closing since the issue is fixed in snakeyaml 1.23.

@yborovikov yborovikov closed this Aug 28, 2018
@asomov
Copy link
Contributor

asomov commented Aug 28, 2018

@yborovikov : Yegor, I would like to make a pull request using the latest SnakeYAML release 1.23 and removing the deprecated methods.

Which branch should I use for this exercise ?

@yborovikov
Copy link
Author

@asomov i'd probably put it in 2.10 - but i'm not the official maintainer.

@asomov
Copy link
Contributor

asomov commented Aug 28, 2018

Created #101

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