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

Integrate CodeStyleTest into StringUtilTest #3907

Merged
merged 1 commit into from
Apr 1, 2018

Conversation

koppor
Copy link
Member

@koppor koppor commented Mar 31, 2018

During #3704, I observed that having many classes causes difficulties in migrating to a multi module build. Since 1) we have ArchUnit in place and 2) the current test in "CodeStyleTest" just covers StringUtils. Therefore, I propose to include the test included there into StringUtilTest.

@stefan-kolb
Copy link
Member

Can be removed after refactoring Stringutils anyway. Also, as we have a Guava dependency now, maybe we can replace a lot of our helper functions?

@koppor
Copy link
Member Author

koppor commented Mar 31, 2018

We checked this approx half a year ago and none of these functions were present in Guava. They are IMHO mostly available in Apache Commons Lang, but our "spirit" wants to remove this well-established library to the more modern Guava library (https://github.com/google/guava/wiki/StringsExplained).

@stefan-kolb
Copy link
Member

Well then you can move it anyway.

@koppor koppor mentioned this pull request Mar 31, 2018
8 tasks
@koppor koppor requested a review from lenhard April 1, 2018 09:23
Copy link
Member

@lenhard lenhard left a comment

Choose a reason for hiding this comment

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

Moving the code is fine, imho.

@lenhard lenhard merged commit 0e4580a into master Apr 1, 2018
@lenhard lenhard deleted the move-codestyle-test-into-stringutils branch April 1, 2018 12:43
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.

3 participants