[#3977] improvement: Add gitignore files of client-python and web into Apache Rat exclusion list #3978
Conversation
| @@ -476,7 +476,7 @@ tasks.rat { | |||
| "clients/client-python/gravitino/utils/http_client.py" | |||
| ) | |||
|
|
|||
| // Add .gitignore excludes to the Apache Rat exclusion list. | |||
| // Add .gitignore of subprojects, make them excludes to the Apache Rat exclusion list. | |||
There was a problem hiding this comment.
Sorry but the original comment makes more sense
| } else { | ||
| exclusions.add("$path/$file") | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Why not just use "**/.gitignore" in the exclusion list?
There was a problem hiding this comment.
It's because some folders would include .gitignore, like node_modules, htmlcov, etc. There are additional rules required to avoid them, so I think it's better to list certain .gitignore manually.
There was a problem hiding this comment.
That would only ignore the .gitignore files not the folders that contain it.
There was a problem hiding this comment.
Rat is just a tool that checks for license headers a .gitignore is very unlikely to have a license header so excluding all of them is fine.
There was a problem hiding this comment.
and even if it does a .giignore file is basically just a list which is not covered by copyright
There was a problem hiding this comment.
Oh, it's not meant to ignore .gitignore file, The .gitignore files listed manually (root, client-python and web, here) are read and parsed to add the files and folders list in them to exclusion list, make Rat to ignore them.
For example, there is venv in .gitignore of client-python project, it will be add in exclusion list so Rat will know that this folder needs not to be checked.
There was a problem hiding this comment.
That's not the best idea idea. Just because a file is listed in .gitignore doesn't mean it should be ignored by rat - although it is likely. It also makes it hard to review a release or easily review what we are excluding from an ASF policy/legal point of view. At the ASF Incubator PMC memebers will manually run rat to verify a release contain no unexpcted licensed files.
There was a problem hiding this comment.
It makes sense, but the original code here works just the same as mine, except it only includes root .gitignore, and that's why I think this idea is acceptable.
There was a problem hiding this comment.
Yep I understod why you did that, but it prbabbly wasn't the best idea to start with for reasons explaned above. I think listing all of the exlustions is probably better so a reviewer can easily see them.
What changes were proposed in this pull request?
build.kts.gradleWhy are the changes needed?
Fix: #3977
Does this PR introduce any user-facing change?
No
How was this patch tested?
./gradlew rat