Fix license header check ignoring well known files #38
Conversation
FranzBusch
force-pushed
the
fb-fix-license
branch
2 times, most recently
from
c727d5a to
598d8d3
Compare
|
We can also enable the license check on this repo after this change to make sure it stays green |
.licenseignore
Outdated
| **/*.yml | ||
| CODEOWNERS | ||
| LICENSE.txt | ||
| README.md |
There was a problem hiding this comment.
Should these files have trailing newlines?
.license_header_template
Outdated
| # See https://swift.org/LICENSE.txt for license information | ||
| # See https://swift.org/CONTRIBUTORS.txt for the list of Swift project authors | ||
| # | ||
| # ===----------------------------------------------------------------------===# |
There was a problem hiding this comment.
What do we need the license header template for? It wasn’t there before
There was a problem hiding this comment.
Because we didn't actually check our license headers here and it appears that the Swift project and our Swift on server packages have slightly different license headers
| if [[ -f .licenseignore ]]; then | ||
| file_paths=$(tr '\n' '\0' < .licenseignore | xargs -0 -I% printf '":(exclude)%" '| xargs git ls-files ":(exclude).licenseignore" ":(exclude).license_header_template" ) | ||
| else | ||
| file_paths=$(git ls-files ":(exclude).license_header_template" ) | ||
| fi |
There was a problem hiding this comment.
The second xargs confused me a little bit until I realized that it only causes git ls-files to be called a single time. Maybe worth pulling out the computation of the exclude list to make it more readable? Something along the lines of the following.
| if [[ -f .licenseignore ]]; then | |
| file_paths=$(tr '\n' '\0' < .licenseignore | xargs -0 -I% printf '":(exclude)%" '| xargs git ls-files ":(exclude).licenseignore" ":(exclude).license_header_template" ) | |
| else | |
| file_paths=$(git ls-files ":(exclude).license_header_template" ) | |
| fi | |
| if [[ -f .licenseignore ]]; then | |
| exclude_list="':(exclude).licenseignore' ':(exclude).license_header_template' $($(tr '\n' '\0' < .licenseignore | xargs -0 -I% printf '":(exclude)%")" | |
| else | |
| exclude_list=":(exclude).license_header_template" | |
| fi | |
| file_paths=$(git ls-files $exclude_list) |
There was a problem hiding this comment.
I agree though I would prefer a follow up PR to unblock the CI on other repos
The previous PR tried to ignore two well known files `.licenseignore` and `.license_header_template`. However, that PR wasn't fully working and broke this check. This PR fixes it by using a slightly different approach for those well known files.
FranzBusch
force-pushed
the
fb-fix-license
branch
from
598d8d3 to
d7354ae
Compare
There was a problem hiding this comment.
Accepting to unblock CI and let’s address https://github.com/swiftlang/github-workflows/pull/38/files#r1805237145 in a follow-up PR.
The previous PR tried to ignore two well known files
.licenseignoreand.license_header_template. However, that PR wasn't fully working and broke this check.This PR fixes it by using a slightly different approach for those well known files.