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

Exposed the serializable-model when generating client models. #907

Merged

Conversation

denvitaharen
Copy link
Contributor

Many thanks for submitting your Pull Request ❤️!

Issue: #869

Exposed the possibility to generate models that implement the java.io.Serilizable-interface. I changed the property from the proposed serializableModel to serializable-model since I thought it harmonized better with the naming of the other configurations.

I wrote an new test module for this, maybe it was a bit over engineered, but I didn't find any existing suitable test to put this in, so I did a new module instead.

  • You have read the contributors guide
  • Your code is properly formatted according to our code style
  • Pull Request title contains the target branch if not targeting main: [0.9.x] Subject
  • Pull Request contains link to the issue
  • Pull Request contains link to any dependent or related Pull Request
  • Pull Request contains description of the issue
  • Pull Request does not include fixes for issues other than the main ticket

@denvitaharen
Copy link
Contributor Author

My bad, I had set IntelliJ to optimize imports and reformat code, which changed everything the verify had made. Rerunned it now and made sured IntelliJ didn't change anything.

@hbelmiro hbelmiro linked an issue Dec 10, 2024 that may be closed by this pull request
Copy link
Contributor

@hbelmiro hbelmiro left a comment

Choose a reason for hiding this comment

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

Thank you @denvitaharen.
Looks great in general. I just left a few minor comments.

@denvitaharen
Copy link
Contributor Author

denvitaharen commented Dec 11, 2024

Thanks for the comments @hbelmiro! I have updated the PR based on your feedback.

Out of curiosity, why isn't star imports ok? I don't have any personal preferences, I just use what's default in my IDE and have never reflected or hade any problems with star imports :)

@hbelmiro
Copy link
Contributor

Thanks for the comments @hbelmiro! I have updated the PR based on your feedback.

Out of curiosity, why isn't star imports ok? I don't have any personal preferences, I just use what's default in my IDE and have never reflected or hade any problems with star imports :)

@denvitaharen star imports don't bring any benefits, but some disadvantages:

  • Reduce readability since it's unclear which package a class comes from
  • Reduce code maintainability: future changes in dependencies or new class additions to imported packages might introduce conflicts or ambiguities

@ricardozanini ricardozanini merged commit 12825ee into quarkiverse:main Dec 12, 2024
7 checks passed
ricardozanini pushed a commit that referenced this pull request Dec 12, 2024
…910)

Co-authored-by: Richard Alm <44467351+denvitaharen@users.noreply.github.com>
@denvitaharen denvitaharen deleted the feature/expose-serializableModel branch December 12, 2024 21:17
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.

Expose serializableModel property as a Quarkus configuration property
3 participants