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

Revert pointless commit to fix #2320 #2349

Merged
merged 2 commits into from
Dec 21, 2017

Conversation

ChristianCiach
Copy link

Revert "use custom object mapper to serialize json in order to avoid
null values"

This reverts commit 3ad7595.

This fixes issue #2320. The original commit is pointless and dangerous,
as it surprisingly overrides the behavior of SwaggerSerializers.java,
but does essentially the same (but without supporting pretty printing).

christianc added 2 commits August 3, 2017 13:02
null values"

This reverts commit 3ad7595.

This fixes issue swagger-api#2320. The original commit is pointless and dangerous,
as it surprisingly overrides the behavior of SwaggerSerializers.java,
but does essentially the same (but without supporting pretty printing).
This fixes issue swagger-api#2320 for YAML documents. The original code
surprisingly overrides the behavior of SwaggerSerializers.java to
needlessly split and join a YAML document. This was originally done to
remove some kind of comment line from the YAML, but this has been
removed long ago, making split/join-code pointless.
@ChristianCiach
Copy link
Author

I've added a second commit to remove a similar issue for YAML documents, where SwaggerSerializers.java has been ignored, too. Also, I've removed some split-and-then-rejoin-code that has been pointless since ages.

@ChristianCiach
Copy link
Author

Is there any reason not to merge this?

@tobilarscheid
Copy link
Contributor

I see the issue with #2320, and I absolutely understand how this commit fixes it. However, your fix now introduces a regression to #2105
/cc @fehguy

@frantuma
Copy link
Member

frantuma commented May 4, 2018

As discussed in related tickets (#2320, #2105, #2106) and specifically in this comment, issue with #2105 is handled by registering SwaggerSerializers; swagger-api/swagger-samples#71 would possibly need to be updated accordingly

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.

None yet

3 participants