-
Notifications
You must be signed in to change notification settings - Fork 54k
BAEL-9305 #18668
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
BAEL-9305 #18668
Conversation
| @Autowired | ||
| private TestOrderRepository orderRepository; | ||
|
|
||
| @Transactional //<-- marks this method |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| @Transactional //<-- marks this method | |
| @Transactional |
I think we should omit this, usually the guideline is to describe in the article rather than in comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
| import jakarta.persistence.Id; | ||
| import jakarta.persistence.Table; | ||
|
|
||
| import lombok.Getter; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a guideline to not use Lombok in articles that aren't specifically demonstrating Lombok features
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great! Very clean
|
|
||
| @Test | ||
| void givenPublicTransactionalMethod_whenCallingIt_thenShouldRollbackOnException() { | ||
| assertThat(testOrderRepository.findAll()).isEmpty(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| assertThat(testOrderRepository.findAll()).isEmpty(); | |
| assertThat(testOrderRepository.findAll()) | |
| .isEmpty(); |
Let's format the chained method calls on separate lines to make them more visible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this really the right way? I applied the Baeldung code style which, after I added line break like you suggested, corrected it back to the way it is now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@LeoHelfferich from the author guidelines:
formatting - fluent APIs
- sometimes, the default formatter does need a bit of manual help
- for example, fluent APIs are more readable if we split each method call on its own line
- if the method call is small enough, and very logically connected to the next one - you can also group 2 on a single line
No description provided.