Skip to content

refactor: erefactor/AutoRefactor - Annotation#8544

Merged
wing328 merged 1 commit intoOpenAPITools:masterfrom
cal101:erefactor/master/AnnotationCleanUp
Feb 8, 2021
Merged

refactor: erefactor/AutoRefactor - Annotation#8544
wing328 merged 1 commit intoOpenAPITools:masterfrom
cal101:erefactor/master/AnnotationCleanUp

Conversation

@cal101
Copy link
Copy Markdown
Contributor

@cal101 cal101 commented Jan 26, 2021

AutoRefactor cleanup 'AnnotationCleanUp' applied by erefactor:

Simplifies annotation uses:

  • empty parentheses will be removed from annotations,
  • single members named "value" will be removed from annotations and
    only the value will be left.

For AutoRefactor see https://github.com/JnRouvignac/AutoRefactor
For erefactor see https://github.com/cal101/erefactor

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.
  • Run the following to build the project and update samples:
    ./mvnw clean package 
    ./bin/generate-samples.sh
    ./bin/utils/export_docs_generators.sh
    
    Commit all changed files.
    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, 5.1.x, 6.0.x
  • If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

@bbdouglas (2017/07) @sreeshas (2017/08) @jfiala (2017/08) @lukoyanov (2017/09) @cbornet (2017/09) @jeff9finger (2018/01) @karismann (2019/03) @Zomzog (2019/04) @lwlee2608 (2019/10) @nmuesch (2021/01)

AutoRefactor cleanup 'AnnotationCleanUp' applied by erefactor:

Simplifies annotation uses:
- empty parentheses will be removed from annotations,
- single members named "value" will be removed from annotations and
only the value will be left.

For AutoRefactor see https://github.com/JnRouvignac/AutoRefactor
For erefactor see https://github.com/cal101/erefactor
@cal101
Copy link
Copy Markdown
Contributor Author

cal101 commented Jan 26, 2021

@wing328 Moin. (Means Hi ;-) Shall I try to split cleanup pull requests on sub project boundaries? What would be some computable criterium to detect the relevant sub project?

@wing328
Copy link
Copy Markdown
Member

wing328 commented Feb 7, 2021

No. I think this is fine. Will review.

Thanks for the PR 👍

@wing328 wing328 added Enhancement: Code Cleanup General refactoring, removal of deprecated things, commenting, etc. Feature: Generator labels Feb 7, 2021
@wing328
Copy link
Copy Markdown
Member

wing328 commented Feb 7, 2021

Quick question: what's the license for erefactor (https://github.com/cal101/erefactor)?

I assume its license is open-source friendly but would like to confirm as I only see a readme in that repo.

@cal101
Copy link
Copy Markdown
Contributor Author

cal101 commented Feb 7, 2021

I never thought about the license on the changes itself.
I added an FAQ entry to erefactor readme to make clear, that erefactor does not claim any rights on the changes.

Which technical committee members shall I address future PRs to if any at all? The Java ones as here?

Do you have plans on reviving/applying the formatter that's configured in the POM file?
I can add that as a erefactor cleanup option and provide a PR on the changes if you like.

There are some cleanups that need a formatting step after application so having a usable formatter is nice to have.

There are some cleanups that depend more on project policy or style and may not fit or maybe
are unwanted for OpenAPI.
I plan to make PRs for them at some time and you are totally free to say "No thanks, we prefer to do it differently."
OK for you?
IMO it's helpful to see the impact of some cleanup instead of just talking about its effect.

If PRs can't be rebased automatically the cleanups can be re-applied on the head, just tell me in the comments.

@wing328
Copy link
Copy Markdown
Member

wing328 commented Feb 8, 2021

cc @OpenAPITools/generator-core-team

@wing328
Copy link
Copy Markdown
Member

wing328 commented Feb 8, 2021

@cal101 thanks for the explanation and the change looks good to me.

@wing328
Copy link
Copy Markdown
Member

wing328 commented Feb 8, 2021

CircleCI failure not related to this change.

@wing328 wing328 merged commit ef9e8b7 into OpenAPITools:master Feb 8, 2021
@wing328 wing328 added this to the 5.0.1 milestone Feb 8, 2021
@cal101 cal101 deleted the erefactor/master/AnnotationCleanUp branch March 24, 2021 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement: Code Cleanup General refactoring, removal of deprecated things, commenting, etc. Feature: Generator

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants