-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
[SHIRO-776] refactor: JUnit5 Best Practices #1338
[SHIRO-776] refactor: JUnit5 Best Practices #1338
Conversation
Use this link to re-run the recipe: https://app.moderne.io/recipes/builder/hBA8IygoJ?organizationId=YzQ4NzM2ZmItNmRjOS00ZmU2LWJhOTUtNDQ4NTViYTRhNWFk Co-authored-by: Moderne <[email protected]>
Thank you for your contribution! |
You're welcome! There's potentially some more automated changes we can do here, but don't want to flood you with PRs. Here's another commit that can be cherry picked: main...timtebeek:shiro:refactor/migrate-to-java-11 |
Go ahead :). Will work through them |
Thanks @timtebeek, this is awesome! Do you have a public Moderne dashboard you can point us to? |
Glad to hear! Yes you can run any of our recipes through https://app.moderne.io/marketplace, which is free for OSS. Or you can select the existing Open Source organization we already have for Apache, Maven, Gradle and others. From there you can go to our marketplace to select and run recipes; I like looking over our best practice recipes. Here's the typical list of recipes to run when first starting out. All of those should complete in about a minute for Shiro, and from there you can then create PRs as I did here. Some recipes also come with dedicated visualizations, which we're wrapping up into a dashboard of insights and fixes to announce coming week. I'll share a link when the blog is out! :) |
As an example of what you could do next, I see you're right now using assertj for one single test: If you were to want to switch over to AssertJ we have a recipe that should do most of the work for you: If not, then it's maybe best to phase out that single use to prevent a long period of dual use or slow adoption. |
Ha, yes. We wanted to switch to assertj at some point, and that test is so far the only one which was added. |
Yes finally I can announce our DevCenter, as posted on our blog: Here's a quick preview of what's explained on the above link We have this available for all of Apache, Gradle, Maven and a few others via https://app.moderne.io/devcenter Needless to say some work to do with regards to the recent Java 17 migration effort for Apache Maven, but exited to help out! :) |
Looks great @timtebeek ! Thanks for the link! |
Following this checklist to help us incorporate your contribution quickly and easily:
for the change (usually before you start working on it). Trivial changes like typos do not
require a GitHub issue. Your pull request should address just this issue, without pulling in other changes.
[#XXX] - Fixes bug in SessionManager
,where you replace
#XXX
with the appropriate GitHub issue. Best practiceis to use the GitHub issue title in the pull request title and in the first line of the commit message.
fixes #XXX
if merging the PR should close a related issue.mvn verify
to make sure basic checks pass. A more thorough check will be performed on your pull request automatically.git rebase -i
.Trivial changes like typos do not require a GitHub issue (javadoc, comments...).
In this case, just format the pull request title like
[DOC] - Add javadoc in SessionManager
.If this is your first contribution, you have to read the Contribution Guidelines
If your pull request is about ~20 lines of code you don't need to sign an Individual Contributor License Agreement
if you are unsure please ask on the developers list.
To make clear that you license your contribution under the Apache License Version 2.0, January 2004
you have to acknowledge this by using the following check-box.
Hi! Noticed that the JUnit 5 migration had missed a few small steps to optimize the methods called, and sometimes swap the argument order. The compiler won't complain about such issues, but it's best to conform to JUnit 5's best practices. Hope this is helpful! It's a mostly automated change, prompted by the summons here: https://twitter.com/lobaorn/status/1763676683012317411
Tagging @bmarwell for review, since he opened
Use this link to re-run the recipe: https://app.moderne.io/recipes/builder/hBA8IygoJ?
organizationId=YzQ4NzM2ZmItNmRjOS00ZmU2LWJhOTUtNDQ4NTViYTRhNWFk