Skip to content

Apply Junit5BestPractices in spring-integration-mail #9946

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

Closed
wants to merge 1 commit into from

Conversation

mjd507
Copy link
Contributor

@mjd507 mjd507 commented Apr 8, 2025

use open-rewrite Junit5BestPractices recipe to rewrite test for spring-integration-mail module

@mjd507
Copy link
Contributor Author

mjd507 commented Apr 8, 2025

discussion: #9947

Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Would you mind to include some details into commit message saying "what" and "why".

Because what I see so far is just removal of public word from those test methods.
While it is convenient to not use it and less words to ready, I find this as silly best practice.
And discussing it takes more time then just leave it as is.
Such a change just don't bring any benefits.

We definitely can accept your change and merge it since you have already dedicated some effort to bring this into life.
But, please, consider in the future to not waste your time for such a pointless change.
Unless that was an automatic pass and we indeed can go ahead and apply it everywhere in the project.

This is not my first discussion about applying OpenRewrite and I think that applying its rules is OK to satisfy community expectations. But that has to be a reasonable amount of the effort.

Thanks for understanding.

@mjd507
Copy link
Contributor Author

mjd507 commented Apr 8, 2025

Totally agreed with you, I will dig more into this further, from effort and benefit perspective.

(actually I didn't waste much time on it, it an auto process, maybe 5mins?, I didn't auto rewrite for all modules is considering too many files changes would be hard for reviewing, if your internal team can do that, that will be great)

thank you again for nice feedback.

  1. remove public keyword for test methods
  2. remove test prefix for test methods

Signed-off-by: Ma,Jiandong <[email protected]>
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.

2 participants