Skip to content
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

fix: use proper computation logic of Kind and API version #2679

Closed

Conversation

metacosm
Copy link
Collaborator

Description

Experiments on trying to improve the generated code. Looking for feedback as well.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • Feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change
  • Chore (non-breaking change which doesn't affect codebase;
    test, version modification, documentation, etc.)

Checklist

  • Code contributed by me aligns with current project license: Apache 2.0
  • I Added CHANGELOG entry regarding this change
  • I have implemented unit tests to cover my changes
  • I have added/updated the javadocs and other documentation accordingly
  • No new bugs, code smells, etc. in SonarCloud report
  • I tested my code in Kubernetes
  • I tested my code in OpenShift

@centos-ci
Copy link

Can one of the admins verify this patch?

@metacosm metacosm self-assigned this Dec 17, 2020
@manusa
Copy link
Member

manusa commented Dec 18, 2020

Is this required for 5.0.0? (not fixing this before release will imply a bug in 5.0.0?)


Haven't checked the purpose of the PR yet. As a global feedback regarding code generation, as stated in this comment (#2160 (comment)) and subsequent conversation, we should try to merge both generators so that we only have to maintain logic at a single place.
Being able to run generators from a platform/environment agnostic perspective should also be a priority (#2662).
Finally improve the shellscripts/makefiles/etc. so that we can run code generation as a single task. Integrate as a Maven profile, etc. are other improvement options.

Relates to:

@metacosm
Copy link
Collaborator Author

I don't think that this is strictly needed for 5.0 because the code is generated in such a way that the current version should work. However, it's worth noting that the logic used to compute these values differs from the one used for "live" classes so it might be a good idea to homogenize that part at some point.

I agree that a more concerted effort on code generation as a whole is probably needed, though.

@manusa
Copy link
Member

manusa commented Mar 3, 2021

Is this PR still relevant?

@metacosm
Copy link
Collaborator Author

metacosm commented Mar 3, 2021

Still relevant in the sense that what this attempts to do should be done at some point but maybe not in this form.

@manusa
Copy link
Member

manusa commented Mar 3, 2021

👍 let's keep it then

@shawkins
Copy link
Contributor

Closing based upon #3327

@shawkins shawkins closed this Aug 14, 2021
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.

5 participants