Skip to content

[dart][dart-dio] Improve prefixing of inner enums with classname - #8220

Closed
kuhnroyal wants to merge 2 commits into
OpenAPITools:5.1.xfrom
kuhnroyal:dart/improve-inner-enum-naming
Closed

[dart][dart-dio] Improve prefixing of inner enums with classname#8220
kuhnroyal wants to merge 2 commits into
OpenAPITools:5.1.xfrom
kuhnroyal:dart/improve-inner-enum-naming

Conversation

@kuhnroyal

Copy link
Copy Markdown
Contributor
  • prevent further name conflicts by correctly naming the enum, until now there could potentially occur conflicts e.g. 2x MapTestInnerEnum by renaming the items child property
  • correctly set enumName to match datatypeWithEnum

The inline enum template has been changed but only enums that were previously called InnerEnum have been changed.
I don't think that was supported before all the recent changes so I don't think this is a breaking change.

PR checklist

  • Read the contribution guidelines.
  • Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
  • If contributing template-only or documentation-only changes which will change sample output, build the project beforehand.
  • Run the shell script ./bin/generate-samples.shto update all Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master. These must match the expectations made by your contribution. You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example ./bin/generate-samples.sh bin/configs/java*. For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

CC @swipesight (2018/09) @jaumard (2018/09) @josh-burton (2019/12) @amondnet (2019/12) @sbu-WBT (2020/12) @kuhnroyal (2020/12) @agilob (2020/12)

@agilob agilob Dec 17, 2020

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

mapTestMapOfEnumStringEnumSerializer 😆 and they say java is verbose

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

 😄 Well to be fair, its is private and generated to prevent collisions

@agilob agilob left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGMT

@kuhnroyal
kuhnroyal force-pushed the dart/improve-inner-enum-naming branch from f047a57 to d14e11f Compare December 17, 2020 14:33
@kuhnroyal

Copy link
Copy Markdown
Contributor Author

@wing328 This is now a breaking change with fallback (model name change?) after the 5.0.0 release and thus needs to go into the 5.1.0 branch?

@wing328

wing328 commented Jan 5, 2021

Copy link
Copy Markdown
Member

@wing328 This is now a breaking change with fallback (model name change?) after the 5.0.0 release and thus needs to go into the 5.1.0 branch?

That's correct. I've updated the target branch to 5.1.x

@wing328
wing328 changed the base branch from master to 5.1.x January 5, 2021 01:41
@kuhnroyal
kuhnroyal force-pushed the dart/improve-inner-enum-naming branch from b5877bf to fa2fa77 Compare January 8, 2021 13:23
* prevent further name conflicts by correctly naming the enum, until now there could potentially occur conflicts e.g. 2x `MapTestInnerEnum` by renaming the `items` child property
* correctly set `enumName` to match `datatypeWithEnum`
@kuhnroyal
kuhnroyal force-pushed the dart/improve-inner-enum-naming branch from fa2fa77 to 6903664 Compare February 2, 2021 00:38
@wing328 wing328 closed this Feb 8, 2021
@wing328
wing328 deleted the branch OpenAPITools:5.1.x February 8, 2021 13:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants