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

Add @SequenceGenerator to make Gift Entity example generally more usable #11728

Merged
merged 1 commit into from Aug 31, 2020
Merged

Add @SequenceGenerator to make Gift Entity example generally more usable #11728

merged 1 commit into from Aug 31, 2020

Conversation

svinther
Copy link

As described in this issue here: #11591

If blindly following the tutorial, it can cause weird sequence behavior in some cases. This pr aims to make the tutorial example usable in general

@gavinking
Copy link

I agree that this is much more correct. I believe that without the @SequenceGenerator annotation, this was illegal JPA code.

@gavinking
Copy link

gavinking commented Aug 29, 2020

I agree that this is much more correct. I believe that without the @SequenceGenerator annotation, this was illegal JPA code.

Except, I don't think you should specify @GeneratedValue(strategy=SEQUENCE) in this case. You don't need it. Just the name of the generator is enough.

@svinther
Copy link
Author

I agree that this is much more correct. I believe that without the @SequenceGenerator annotation, this was illegal JPA code.

Except, I don't think you should specify @GeneratedValue(strategy=SEQUENCE) in this case. You don't need it. Just the name of the generator is enough.

I am fine with removing that part, but in this way it is in sync with the hibernate-orm quickstart https://github.com/quarkusio/quarkus-quickstarts/blob/master/hibernate-orm-quickstart/src/main/java/org/acme/hibernate/orm/Fruit.java#L20 ?

@jaikiran jaikiran requested a review from Sanne August 29, 2020 11:15
@gavinking
Copy link

Well I think it would be better to change both. We shouldn't give people the idea that they need to write code that is more verbose than necessary.

It's unfortunate that the Javadoc for the annotation actually has a code example with the same redundancy.

@svinther
Copy link
Author

svinther commented Aug 29, 2020

I have updated this PR, removing the verbose jpa annotations.

About the quickstarts, There are a few places where:

  1. strategy=SEQUENCE is used along with generator=XX
  2. strategy=AUTO (being the default value so can be omitted)

All of those can be removed as unnecessary verbosity, like this https://github.com/svinther/quarkus-quickstarts/commit/4231c1af9de754d4d30ba6d4157cdb95c24c84ba ?

@gastaldi
Copy link
Contributor

Nice! May I ask you to squash your commits before qe merge this? Thank you!

@svinther
Copy link
Author

@gastaldi commits squashed

Copy link
Contributor

@gastaldi gastaldi left a comment

Choose a reason for hiding this comment

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

Nice! Thank you!

@gastaldi gastaldi merged commit e2ad6a6 into quarkusio:master Aug 31, 2020
@svinther svinther deleted the fix_hibernate_entity_seq branch August 31, 2020 16:53
@gsmet gsmet added this to the 1.8.0.CR1 milestone Sep 2, 2020
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.

None yet

5 participants