Here are some quick notes for skywalking-eyes, hoping them will be helpful.
Before using skywalking-eyes, we need to setup config as said in [1]. Take iceberg-rust as an example [2]. For checking in CI: Adding following content in workflow [3] - name: Check License Header uses: apache/skywalking-eyes/header@v0.5.0 For local usage: docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header check docker run -it --rm -v $(pwd):/github/workspace apache/skywalking-eyes header fix [1]: https://github.com/apache/skywalking-eyes?tab=readme-ov-file#configurations [2]: https://github.com/apache/iceberg-rust/blob/main/.licenserc.yaml [3]: https://github.com/apache/iceberg-rust/blob/94a1c5d7742bc3b2a9ac7c8da20711a5e2578b89/.github/workflows/ci.yml#L38C1-L39C51 On Fri, Oct 27, 2023, at 22:17, Jean-Baptiste Onofré wrote: > Thanks for the heads up Xuanwo. > > It's the fourth option :) I will make a comparison with RAT. > > Regards > JB > > On Fri, Oct 27, 2023 at 12:15 PM Xuanwo <xua...@apache.org> wrote: >> >> iceberg-rust is using apache/skywalking-eyes/header@v0.5.0 now. >> >> BTW, we found skywalking-eyes works really well. It's fast, correct and >> well-maintained. >> >> Maybe worth take a look. >> >> On Fri, Oct 27, 2023, at 17:48, Jean-Baptiste Onofré wrote: >> > By the way, as dev/check-license is also used in iceberg-python and >> > iceberg-go repositories (iceberg-rust doesn't have it), maybe I can >> > move forward on new rat release with the fix on hidden directories and >> > update there as well. >> > >> > Regards >> > JB >> > >> > On Thu, Oct 26, 2023 at 5:19 PM Jean-Baptiste Onofré <j...@nanthrax.net> >> > wrote: >> >> >> >> Hi guys, >> >> >> >> During the 1.4.1 vote, we identified some files without ASF headers, >> >> more specifically in hidden directories (like .baseline). These files >> >> have not been detected by dev/check-license script. >> >> >> >> The reason is because the script uses apache-rat via java -jar (the >> >> Apache RAT CLI), executing the RAT Report class () >> >> >> >> When providing a directory to scan, the Report class uses a >> >> DirectoryWalker to traverse the directories, looking for files to >> >> check. >> >> Unfortunately, by default, the DirectoryWalker ignores the hidden >> >> directory (all directories starting with .): >> >> >> >> https://github.com/apache/creadur-rat/blob/master/apache-rat-core/src/main/java/org/apache/rat/walker/DirectoryWalker.java#L71 >> >> >> >> https://github.com/apache/creadur-rat/blob/master/apache-rat-core/src/main/java/org/apache/rat/walker/Walker.java#L53 >> >> >> >> In our case, it means that it ignores .baseline, .github, .git, >> >> .palantir directories. This is not good as .baseline, .github and >> >> .palantir directories are included in our source distribution, so the >> >> ASF headers should be clean here. >> >> >> >> FYI, I will propose a change in rat to, at least, be able to use a >> >> ReportConfiguration to define if we want to restrict directories or >> >> not (be able to configure the Walkers basically). I will try to >> >> include it in rat 0.17 (I discussed with Claude about that). >> >> NB: rat gradle and maven plugins define their own Walkers/IReportable >> >> to avoid this issue. >> >> >> >> I propose three options to improve this: >> >> 1. We keep dev/check-license as it is today (using rat 0.15) and we >> >> exclude .baseline, .github, .palantir directories from our source >> >> distribution. It means that we will probably have issues while >> >> building from source distribution. We can revisit dev/check-license >> >> when we upgrade to rat 0.17 >> >> >> >> 2. We keep dev/check-license but we create our own rat scanner with a >> >> custom IReportable class considering all files/directories. Something >> >> like: >> >> >> >> public static void main(String[] args) throws Exception { >> >> ReportConfiguration reportConfiguration = new >> >> ReportConfiguration(); >> >> >> >> reportConfiguration.setHeaderMatcher(Defaults.createDefaultMatcher()); >> >> >> >> XmlWriter writer = new XmlWriter(new FileWriter("report.xml")); >> >> ClaimStatistic claimStatistic = new ClaimStatistic(); >> >> RatReport ratReport = XmlReportFactory.createStandardReport( >> >> writer, >> >> claimStatistic, >> >> reportConfiguration >> >> ); >> >> ratReport.startReport(); >> >> IncludeHiddenDirectoryWalker walker = new >> >> IncludeHiddenDirectoryWalker(new File("/path/to/iceberg")); >> >> walker.run(ratReport); >> >> ratReport.endReport(); >> >> writer.closeDocument(); >> >> } >> >> >> >> public class IncludeHiddenDirectoryWalker implements IReportable { >> >> >> >> >> >> private File file; >> >> >> >> public IncludeHiddenDirectoryWalker(File file) { >> >> this.file = file; >> >> } >> >> >> >> @Override >> >> public void run(RatReport report) throws RatException { >> >> process(report, file); >> >> } >> >> >> >> public void process(RatReport report, File file) { >> >> final File[] files = file.listFiles(); >> >> if (files != null) { >> >> for (File current : files) { >> >> if (current.isDirectory()) { >> >> process(report, current); >> >> } else { >> >> try { >> >> Document document = new FileDocument(current); >> >> report.report(document); >> >> } catch (RatException e) { >> >> System.err.println("Can't report file " + >> >> current.getAbsolutePath() + ": " + e); >> >> } >> >> } >> >> } >> >> } >> >> } >> >> >> >> } >> >> >> >> So, I can contribute this in dev/src for example. >> >> >> >> 3. Instead of using dev/check-license, we can use the rat gradle >> >> plugin (https://github.com/eskatos/creadur-rat-gradle/). I tested it >> >> and it works fine as it uses a custom IReportable like: >> >> >> >> https://github.com/eskatos/creadur-rat-gradle/blob/master/src/main/kotlin/org/nosphere/apache/rat/RatWork.kt#L135 >> >> >> >> https://github.com/eskatos/creadur-rat-gradle/blob/master/src/main/kotlin/org/nosphere/apache/rat/RatWork.kt#L189 >> >> >> >> We can include this plugin the check gradle phase, meaning that we can >> >> verify headers for each PR. >> >> >> >> My preference would be for 3, mainly because: >> >> 1. it integrates smoothly in our gradle ecosystem, adding a new plugin >> >> as we have gradle-baseline-java, gradle-errorprone-plugin, >> >> spotless-plugin-gradle, etc >> >> 2. As we can hook rat gradle plugin in the gradle check task, it means >> >> license check will be perform at build time, including check on PR by >> >> GitHub Actions >> >> >> >> If you are OK with 3, I will work on: >> >> 1. a PR to use it >> >> 2. a PR for website to update release check procedure >> >> >> >> Thoughts ? >> >> >> >> Regards >> >> JB >> >> -- >> Xuanwo -- Xuanwo
