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

refactor: Remove public visibility of JUnit 5 tests #3641

Conversation

timtebeek
Copy link
Contributor

Another round of removing public modifiers from tests not needed anymore in JUnit JUpiter, after #3630 (comment).
This helps the public modifiers that are there for a reason stand out, and stops proliferation elsewhere.

Probably best to squash merge this one; had to revert some formatting changes.

@timtebeek
Copy link
Contributor Author

There's also some common static analysis issues that could be cleaned up, but I was doubting if that's appreciated as it does switch some stylistic choices perhaps.

Copy link
Member

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

Thanks yet again. We always squash on way in if multiple commits, so no problem there.

@timtebeek
Copy link
Contributor Author

And of course if you'd want to run any of these same recipes against other openzipkin repositories I can facilitate that through the platform; just let me know and I can setup an OpenZipkin organization to select there.

@codefromthecrypt
Copy link
Member

@timtebeek wrt OpenZipkin organization. Thanks, it would be helpful. Once we release 2.25.0 here we'll go to zipkin-reporter-java and do the same things notably off junit 4. Moderne is already proving a good decrufting tool, particularly as we are free to use offline and leave no side effects in the build.

@codefromthecrypt codefromthecrypt merged commit 6bfc39b into openzipkin:master Dec 13, 2023
10 checks passed
@timtebeek timtebeek deleted the refactor/remove-public-visibility-of-j-unit-5-tests branch December 13, 2023 10:17
@timtebeek
Copy link
Contributor Author

Ah perfect, you can now select OpenZipkin from top left corner and run recipes against all of these repositories
image

Here's first run of JUnit Jupiter best practices against among others the zipkin-reporter-java that you mentioned:
https://app.moderne.io/results/YiPSV5epL

You can then follow up with the AssertJ best practices, Logging best practices and any other recipes you feel might improve the code there. Hope that helps!

@codefromthecrypt
Copy link
Member

Thanks @timtebeek and just in time!

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