Skip to content

chore: refactor date library usage to use CodegenConstants.DATE_LIBRARY#12695

Open
ChihweiLHBird wants to merge 1 commit intoswagger-api:masterfrom
ChihweiLHBird:move-date-lib-constant-to-shared-file
Open

chore: refactor date library usage to use CodegenConstants.DATE_LIBRARY#12695
ChihweiLHBird wants to merge 1 commit intoswagger-api:masterfrom
ChihweiLHBird:move-date-lib-constant-to-shared-file

Conversation

@ChihweiLHBird
Copy link

@ChihweiLHBird ChihweiLHBird commented Mar 24, 2026

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh and ./bin/security/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: 3.0.0 branch for changes related to OpenAPI spec 3.0. Default: master.

Description of the PR

Since this may be considered as a breaking change, I would like to leave the decision making to the team.

  • Move DATE_LIBRARY to CodegenConstants as it is shared across
    the Java and Kotlin language families
  • Add @Deprecated aliases in AbstractJavaCodegen and
    KotlinClientCodegen for backward compatibility

Copilot AI review requested due to automatic review settings March 24, 2026 19:14
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Refactors Java/Kotlin codegen option handling so dateLibrary is sourced from the shared CodegenConstants.DATE_LIBRARY, reducing duplication across language families and aligning option keys.

Changes:

  • Introduce CodegenConstants.DATE_LIBRARY and update generators/tests to use it.
  • Add @Deprecated aliases for DATE_LIBRARY in AbstractJavaCodegen and KotlinClientCodegen.
  • Remove AbstractJavaCodegen’s locally-declared WITH_XML constant and switch internal usage to CodegenConstants.WITH_XML.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
modules/swagger-codegen/src/test/java/io/swagger/codegen/options/KotlinClientCodegenOptionsProvider.java Update test options to use CodegenConstants.DATE_LIBRARY.
modules/swagger-codegen/src/test/java/io/swagger/codegen/options/JaxRSServerOptionsProvider.java Update test options to use CodegenConstants.DATE_LIBRARY / CodegenConstants.WITH_XML.
modules/swagger-codegen/src/test/java/io/swagger/codegen/options/JavaOptionsProvider.java Update test options to use CodegenConstants.DATE_LIBRARY / CodegenConstants.WITH_XML.
modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/SpringCodegen.java Switch dateLibrary defaulting check to CodegenConstants.DATE_LIBRARY.
modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/KotlinClientCodegen.java Deprecate local DATE_LIBRARY constant and use CodegenConstants.DATE_LIBRARY throughout.
modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/AbstractJavaCodegen.java Deprecate local DATE_LIBRARY, remove local WITH_XML, and use CodegenConstants keys internally.
modules/swagger-codegen/src/main/java/io/swagger/codegen/CodegenConstants.java Add shared DATE_LIBRARY constant.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@ChihweiLHBird ChihweiLHBird marked this pull request as draft March 24, 2026 19:27
@ChihweiLHBird ChihweiLHBird force-pushed the move-date-lib-constant-to-shared-file branch 2 times, most recently from 3f03898 to 9195974 Compare March 24, 2026 19:36
@ChihweiLHBird ChihweiLHBird requested a review from Copilot March 24, 2026 19:36
@ChihweiLHBird ChihweiLHBird marked this pull request as ready for review March 24, 2026 19:36
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Signed-off-by: Zhiwei Liang <zhiwei.liang@zliang.me>
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

modules/swagger-codegen/src/main/java/io/swagger/codegen/languages/AbstractJavaCodegen.java:65

  • WITH_XML is still declared locally in AbstractJavaCodegen even though CodegenConstants.WITH_XML already exists. This keeps the shadowing/duplication the PR description says was removed, and makes it easy for the two definitions to drift. Consider removing the local WITH_XML constant and switching its usages to CodegenConstants.WITH_XML (or, if you need backward compatibility for external references, keep a deprecated alias similar to DATE_LIBRARY).
    public static final String DEFAULT_LIBRARY = "<default>";
    public static final String JAVA8_MODE = "java8";
    public static final String JAVA11_MODE = "java11";
    public static final String SUPPORT_ASYNC = "supportAsync";
    public static final String WITH_XML = "withXml";

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 7 out of 7 changed files in this pull request and generated no new comments.


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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.

2 participants