-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
[dart] better date format #6389
Conversation
73dda7a
to
04c0a02
Compare
Guys, I don't know why the CI builds fail. I re-updated the samples with the new instructions. I used the git bash on windows to run |
LGTM but would like to see a test for it |
# Conflicts: # modules/openapi-generator/src/main/resources/dart2/apilib.mustache # modules/openapi-generator/src/main/resources/dart2/pubspec.mustache # samples/client/petstore/dart2/petstore_client_lib/lib/api.dart # samples/client/petstore/dart2/petstore_client_lib/pubspec.yaml
I'll take a look in the coming week. |
@@ -1,6 +1,7 @@ | |||
part of {{pubName}}.api; | |||
|
|||
const _delimiters = const {'csv': ',', 'ssv': ' ', 'tsv': '\t', 'pipes': '|'}; | |||
final _dateFormatter = DateFormat('yyyy-MM-dd'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Am I understanding it correctly that _dateFormatter cannot be changed to other date formats during runtime? Shall we make it configurable at runtime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thoughts were to protect it against accidentally change. I don't see any reasons to allow modification as the specification says it hast to be this format https://swagger.io/specification/#dataTypes. But if you wish, I can change it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally, all REST API should be using the date format following RFC3339 according to OpenAPI spec but there were use cases before in which date using a different format and that's why we make it configurable, e.g. C# configuration class: https://github.com/OpenAPITools/openapi-generator/blob/master/samples/client/petstore/csharp/OpenAPIClient/src/Org.OpenAPITools/Client/Configuration.cs#L100
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay, I will remove the final
c792874
to
b3f6612
Compare
Everything is fine now, isn't it? @wing328 |
@sbu-WBT PR merged. Thanks for your contribution. |
* master: (720 commits) [docs] Update README badges (#7276) Update apiInvoker.mustache and sample file for akka-scala client for issue #7258 fix (#7259) [Dart] Get all enum values in a list (#7166) Update .gitattributes [ci] Set ubuntu workflow verification to autoclrf=true, safeclrf=false Update check-supported-versions.yaml [ci] Update gitattributes and allow skipping docs generation for Windows CI workflows (#7273) [core][bug] FILES is now path relative with no prefixes (#7271) Update check-supported-versions.yaml Update check-supported-versions.yaml (#7268) [Java][jersey2] Add jersey injection dependencies (#7240) [C][Clang Static Analyzer] Remove the useless variable when assembling URL (#7255) Date format dart (#6389) minor enhancement to java client generator (#7253) typescript: Fix Union Types Import Issue (#6789) Modifying the es5 and es6 templates for javascript to handle default values (#6649) [python-exp] simplify examples (#7157) Support for KumuluzEE microprofile runtime (#5944) [C#][netcore] minor improvements and bug fixes (#7244) Deprecate Flash (ActionScript) client generator (#7231) ...
* master: (219 commits) [java] Appropriate instantiation of model with dynamic properties (OpenAPITools#6052) [docs] Update README badges (OpenAPITools#7276) Update apiInvoker.mustache and sample file for akka-scala client for issue OpenAPITools#7258 fix (OpenAPITools#7259) [Dart] Get all enum values in a list (OpenAPITools#7166) Update .gitattributes [ci] Set ubuntu workflow verification to autoclrf=true, safeclrf=false Update check-supported-versions.yaml [ci] Update gitattributes and allow skipping docs generation for Windows CI workflows (OpenAPITools#7273) [core][bug] FILES is now path relative with no prefixes (OpenAPITools#7271) Update check-supported-versions.yaml Update check-supported-versions.yaml (OpenAPITools#7268) [Java][jersey2] Add jersey injection dependencies (OpenAPITools#7240) [C][Clang Static Analyzer] Remove the useless variable when assembling URL (OpenAPITools#7255) Date format dart (OpenAPITools#6389) minor enhancement to java client generator (OpenAPITools#7253) typescript: Fix Union Types Import Issue (OpenAPITools#6789) Modifying the es5 and es6 templates for javascript to handle default values (OpenAPITools#6649) [python-exp] simplify examples (OpenAPITools#7157) Support for KumuluzEE microprofile runtime (OpenAPITools#5944) [C#][netcore] minor improvements and bug fixes (OpenAPITools#7244) ...
Fixes #2696
If something is missing or other changes have to be done, I'm open for any suggestions.
I'm not sure if the petstore changes are correct. I executed all commands from dart-petstore.bat
@ircecho @swipesight @jaumard @athornz @amondnet
PR checklist
./bin/
(or Windows batch scripts under.\bin\windows
) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run./bin/{LANG}-petstore.sh
,./bin/openapi3/{LANG}-petstore.sh
if updating the code or mustache templates for a language ({LANG}
) (e.g. php, ruby, python, etc).master
,4.3.x
,5.0.x
. Default:master
.