Skip to content

[PHP] Include microseconds in serialized date-time#4542

Merged
wing328 merged 2 commits intoOpenAPITools:masterfrom
hinrik:php_serialize_milliseconds
Nov 21, 2019
Merged

[PHP] Include microseconds in serialized date-time#4542
wing328 merged 2 commits intoOpenAPITools:masterfrom
hinrik:php_serialize_milliseconds

Conversation

@hinrik
Copy link
Copy Markdown
Contributor

@hinrik hinrik commented Nov 19, 2019

@hinrik hinrik changed the title [PHP] Include milliseconds in serialized date-time [PHP] Include microseconds in serialized date-time Nov 19, 2019
@wing328 wing328 added this to the 4.2.2 milestone Nov 20, 2019
@wing328
Copy link
Copy Markdown
Member

wing328 commented Nov 20, 2019

CI failed with the following error:

........F........................................................ 65 / 69 ( 94%)
....                                                              69 / 69 (100%)
Time: 1.6 seconds, Memory: 10.00 MB
There was 1 failure:
1) OpenAPI\Client\DateTimeSerializerTest::testDateTimeSanitazion
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'1973-04-30T17:05:00.000+02:00'
+'1973-04-30T17:05:00+02:00'
/home/travis/build/OpenAPITools/openapi-generator/samples/client/petstore/php/OpenAPIClient-php/tests/DateTimeSerializerTest.php:20

Can we make the datetime format configurable similar to what we've done in other languages?

e.g. C# client: https://github.com/OpenAPITools/openapi-generator/blob/master/samples/client/petstore/csharp/OpenAPIClient/src/Org.OpenAPITools/Client/Configuration.cs#L338

@ackintosh
Copy link
Copy Markdown
Contributor

ackintosh commented Nov 20, 2019

I think making the format configurable is good idea. It could be available with adding the format into ObjectSerializer's static property.

ObjectSerializer::setDateTimeFormat(\DateTime::RFC3339_EXTENDED);

@ackintosh
Copy link
Copy Markdown
Contributor

ackintosh commented Nov 20, 2019

https://ci.appveyor.com/project/WilliamCheng/openapi-generator/builds/28988450

fatal: unable to access 'https://github.com/wing328/swagger-samples/': Failed to connect to github.com port 443: Timed out

@wing328 Could you retry the job?

wing328
wing328 approved these changes Nov 21, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@wing328 wing328 left a comment

Choose a reason for hiding this comment

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

LGTM. All tests passed.

@wing328 wing328 merged commit eebad5c into OpenAPITools:master Nov 21, 2019
@hinrik hinrik deleted the php_serialize_milliseconds branch November 21, 2019 07:22
@wing328
Copy link
Copy Markdown
Member

wing328 commented Dec 2, 2019

@hinrik thanks for the PR, which has been included in the v4.2.2 release: https://twitter.com/oas_generator/status/1201432648544972800

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wing328 wing328 wing328 approved these changes

Assignees

No one assigned

Labels

Client: PHP Enhancement: Compatibility General: Awaiting feedback

Projects

None yet

Milestone

4.2.2

Development

Successfully merging this pull request may close these issues.

3 participants

@hinrik @wing328 @ackintosh