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

Remove dependency on apache commons validator #448

Closed
sachin-walia opened this issue Aug 16, 2016 · 6 comments
Closed

Remove dependency on apache commons validator #448

sachin-walia opened this issue Aug 16, 2016 · 6 comments

Comments

@sachin-walia
Copy link

Apache commons validator pulls in it's own dependency tree:

commons-validator:commons-validator:1.5.0
commons-beanutils:commons-beanutils:1.9.2
commons-digester:commons-digester:1.8.1
commons-collections:commons-collections:3.2.2

Upon looking at the code I see that commons validator is only used for

import org.apache.commons.validator.routines.InetAddressValidator;

I think it may be worthwhile pulling this method in from commons validator and getting rid of this dependency tree as apache commons collections 3.x is no longer maintained since 4.1 is already out.

@harshavardhana
Copy link
Member

👍 @sachinwalia2k8

@sachin-walia
Copy link
Author

@harshavardhana I forked this repo and started making this change. The code has dependency on InetAddressValidator which in turn has dependency on RegexValidator. I shaded these classes to io.minio.org.apache.commons.validator.routines.

However I noticed some issues in building the code on Intellij. This is primarily happening due to classes that are residing in policy folder doesn't have "policy" in their package names. I understand JDK would still compile the classes even if they are not in their correct directory layout however most of the tools will have problem. If you want I can send the pull request with all these changes but please understand that this will require regenerating Javadoc for your website.

@balamurugana
Copy link
Member

@sachinwalia2k8 please go ahead and send the fix 👍

@sachin-walia
Copy link
Author

I have made the changes and will be checking-in tonight. There are some breaking changes where some existing classes visibility needs to be upgraded since we are keeping them in policy package. I am hitting a checkstyle issue for Apache Validator classes. Once done I'll send a pull request tonight.

@sachin-walia
Copy link
Author

pull request #449 sent just now.

@sachin-walia
Copy link
Author

Closing this issue since pull request #449 is merged.

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

No branches or pull requests

3 participants