-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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
[MINOR] Fix apache-rat violations #1639
Conversation
Codecov Report
@@ Coverage Diff @@
## master #1639 +/- ##
============================================
+ Coverage 16.71% 16.72% +0.01%
- Complexity 795 796 +1
============================================
Files 340 340
Lines 15030 15030
Branches 1499 1499
============================================
+ Hits 2512 2514 +2
+ Misses 12188 12186 -2
Partials 330 330
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jfrazee for fixing the rat violations. I looked around old emails and previous PRs which brought this change. I think it is enough to just mention the attribution in NOTICE file.
For the other classes, the underlying problem is that apache-rat was not enabled for hudi-utilities bundle |
@jfrazee : Thanks a lot for your help in identifying and providing the fix. There is one more related change as part of this and I took the liberty to add the patch to this PR. If the tests goes fine, I will merge the combined changes. |
@bvaradar |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1 to merge
42c19ca
to
459e7e0
Compare
@vinothchandar : Added rat check to hudi-integ-test. |
* MINOR Fix apache-rat violations. Also, enabling RAT for hudi-utilities and hudi-integ-test
This fixes a few apache-rat violations and adds exclusions for the GitHub PR template type stuff. Note there is already a general attribution for Twitter in the NOTICE so I don't think we need to add another.