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

add trimToNull method on GrailsStringUtils #13807

Open
wants to merge 1 commit into
base: 7.0.x
Choose a base branch
from

Conversation

jamesfredley
Copy link
Contributor

@jamesfredley jamesfredley commented Oct 23, 2024

Enables api 'org.apache.commons:commons-lang3:3.17.0' to be removed from grails-database-migration.

https://github.com/search?q=repo%3Agrails%2Fgrails-database-migration%20StringUtils&type=code

Copy link
Contributor

@codeconsole codeconsole left a comment

Choose a reason for hiding this comment

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

Thanks, I was going to do this, but I would also like a comment that references commons-lang and the version it was taken from and even a permalink to the public repo.

Although it might not be critical for this method, there is a need to bring some other methods over and it is best they keep in sync with the original repo.

In the end, I want to search for "commons-lang" or "commons-text" and easily find this method. This probably should actually be referenced to commons-text if it was moved there.

The end goal is to get rid of commons-lang3 and commons-text completely.

@jamesfredley
Copy link
Contributor Author

Ok, make any changes you would like.

This method does the same thing, but the implementation is different, utilizing org.springframework.util.StringUtils.hasLength()

@codeconsole
Copy link
Contributor

ok, thanks, still would be useful. It's too bad commons-text depends on commons-lang3. If the 2 dependencies didn't take up so much disk space, I would support keeping them, but I don't see the few methods that we actually use warranting the amount of disk space.

@jeffscottbrown
Copy link
Member

jeffscottbrown commented Oct 28, 2024

This method does the same thing, but the implementation is different, utilizing org.springframework.util.StringUtils.hasLength()

Do you know if the commons-lang method has test coverage for any surprising cases (don't know what they might be)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

3 participants