Skip to content
New issue

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

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

Already on GitHub? # to your account

Consider java.time.OffsetDateTime a Swagger DateTime primitive #1759

Merged
merged 1 commit into from
Dec 19, 2016
Merged

Consider java.time.OffsetDateTime a Swagger DateTime primitive #1759

merged 1 commit into from
Dec 19, 2016

Conversation

acourtneybrown
Copy link

No description provided.

@Hades32
Copy link

Hades32 commented Jul 19, 2016

@fehguy why hasn't this been merged yet?

@fehguy
Copy link
Contributor

fehguy commented Jul 19, 2016

Because there's too much to do :) I don't just merge without testing, and I should get to this shortly. Perhaps @frantuma can review faster than me.

@Hades32
Copy link

Hades32 commented Jul 21, 2016

Good to know, I already feared there might be valid reasons to refuse it.

It's really strange how the swagger online editor and cli generate Java8 time types, but the swagger UI doesn't properly support it...

@webron
Copy link
Contributor

webron commented Jul 22, 2016

@Hades32 doesn't support it how?

@Hades32
Copy link

Hades32 commented Jul 23, 2016

@webron it's not shown as a timestamp but instead a large json object with many fields. so pretty useless...

@fehguy
Copy link
Contributor

fehguy commented Jul 25, 2016

@webron I don't see why we wouldn't do this--but we do need to add some tests before merging, or it'll get lost.

@webron
Copy link
Contributor

webron commented Jul 25, 2016

All I was asking is how this is seen in swagger-ui as some people expect it to be something other than a string. It definitely shouldn't appear as a multi-field object.

@danny-zegel-zocdoc
Copy link

@fehguy @webron any ETA on merging this?

@Helmsdown
Copy link
Contributor

+1 for this PR

@Helmsdown
Copy link
Contributor

@webron This is how it renders...
image

@Helmsdown
Copy link
Contributor

@webron @fehguy if it means anything....I vouch for this change it is almost identical to the change I made to support ZonedDateTime. Unfortunately, you can't add tests because the project compiles for java 6 (assuming that has not changed).

#1715

@xunnanxu
Copy link

xunnanxu commented Dec 6, 2016

+1 for this

@dwhu
Copy link

dwhu commented Dec 6, 2016

+1

@fehguy fehguy merged commit 065f6b1 into swagger-api:master Dec 19, 2016
@fehguy fehguy modified the milestone: v1.5.11 Dec 19, 2016
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants