I noticed that there was an inversion inside checkForAliasCircle, which was using hasAlias(alias, name). However, the method signature should be hasAlias(String name, String alias), so I corrected it. The tests didn't break, so I took the opportunity to increase the test coverage for untested methods.
Comment From: sbrannen
Hi @e-freni,
Congratulations on submitting your first PR on GitHub "ever"! 👍
Unfortunately, the changes you have proposed constitute a "breaking change" (in terms of logic) and also break the build -- for example, org.springframework.beans.factory.DefaultListableBeanFactoryTests.aliasCircle() fails after your change.
In addition, there are several Checkstyle violations that break the build.
In light of that, I am closing this PR.
If you would like to modify this PR (or submit a new PR) to increase the code coverage for the existing functionality in SimpleAliasRegistry, feel free to do so.
Please ensure that you run a complete build before submitting a PR.
Thanks
Comment From: e-freni
Hi @sbrannen ! Thank you very much for the warm welcome, your feedback, and for taking the time to review my pull request. I really appreciate it.
I apologize for not having done a full build before submitting the PR. Also, I had never encountered the styling enforced on the build, but I do value it greatly.
I usually work with Maven, but I'm still getting accustomed to Gradle. I'll make sure to improve in that area.
Once again, thank you for your input, and I'll make the necessary adjustments to the pull request. If you have any further suggestions or feedback in the future, please don't hesitate to share.