chore: licence compliance for published artifacts - #51
Conversation
dskarzh
left a comment
dskarzh
left a comment
There was a problem hiding this comment.
Review summary
Reviewed 90 changed files in chore: licence compliance for published artifacts. Left 1 comment inline.
Correctness pass (quality lens skipped per reviewer request). Verified against the worktree: license:check passes with the new validate-phase binding; ${project.inceptionYear} resolves (2022, pom line 23); exactly 16 files carry the ThingsBoard header and they match the include list one-to-one; all 16 are absent from upstream MVEL (checked upstream tree and fork git history — BreakNode/SwitchNode were added post-fork in 2023, so replacing their MVEL headers is correct); exactly 69 files carry the modification notice, matching the description; the vendored ASM sources do say 2000-2011, confirming the ASM-LICENSE correction; both licence files are LF-normalized with trailing newlines; no stale references to the old .txt filenames remain; no lingering test-jar goals in the pom.
This review was auto-generated. Findings may contain errors — please verify before applying changes.
dskarzh
left a comment
dskarzh
left a comment
There was a problem hiding this comment.
Re-review summary
Re-reviewed chore: licence compliance for published artifacts — verified 1 finding from previous review.
| Status | Count |
|---|---|
| ✅ Resolved | 1 |
| 💬 Acknowledged | 0 |
All previous findings have been addressed.
Finding details
- ✅ pom.xml:297 — javadoc jar shipped without the licence texts — Fixed in code:
maven-resources-pluginnow stages both texts intotarget/apidocs/META-INFatprepare-package, beforejavadoc:jarpackages the directory. Verified with a clean build of the PR head: all three published jars containMETA-INF/LICENSEandMETA-INF/ASM-LICENSE, with the javadoc HTML intact.
The fix commit was also reviewed for new issues (correctness pass, quality lens skipped per reviewer request) — none found.
This re-review was auto-generated. Findings may contain errors — please verify before applying changes.
f7395e9 to
471fac6
Compare
dskarzh
left a comment
dskarzh
left a comment
There was a problem hiding this comment.
Review summary
Reviewed 90 changed files in chore: licence compliance for published artifacts. No correctness or safety issues found.
Correctness pass only (quality/design skipped per reviewer request). I did not re-flag the resolved javadoc-jar licence discussion. A clean package produced the main, sources, and javadoc TBEL jars with both licence texts in each and no published test jars; the full suite passed with 1,369 tests.
This review was auto-generated. Findings may contain errors — please verify before applying changes.
c2aaa8a to
6d89b9d
Compare
- ship the MVEL and ASM license texts in the published artifacts, including the javadoc jar - stop publishing the test and test-sources jars - mark the files modified from upstream, with the copyright line in the SPDX tag format - stamp the ThingsBoard-authored files with SPDX license headers, enforced by license-maven-plugin at build time - add a README and fix the hamcrest license name in a test attribution comment - fix the scm urls, use https for the license url, and declare the license by its SPDX name in the pom
6d89b9d to
e8edb7f
Compare
1 participant
Licence hygiene for the published artifacts, plus the version bump needed to release it.
Changes
Ship the licence texts. The jar carried neither the Apache-2.0 text nor the BSD notice covering the vendored ASM copy under
org/mvel2/asm, though both files were in the repository. They are now copied intoMETA-INF, so they travel with every published artifact. Also dropped the.txtextensions, normalised both files to LF with a trailing newline, and corrected the ASM copyright range to2000-2011to match the vendored sources (upstream had left it at2000-2005).Stop publishing the
testsandtest-sourcesjars. Both executions were inherited from upstream MVEL rather than chosen. Nothing consumes either artifact, and each is a further published copy that has to carry its own licence texts.Mark the MVEL files we have modified. TBEL alters 69 upstream files, none of which said so. Each now carries a short notice below the existing upstream header, which is left untouched. The notice points at the Git history rather than describing individual changes, so it needs no maintenance.
Manage ThingsBoard licence headers with
license-maven-plugin. The 16 files ThingsBoard authored had no header; two of them (BreakNode,SwitchNode) carried MVEL's header instead, crediting MVEL's authors with code written here in 2023 — neither file exists upstream. Headers are now generated from the same template and settings the AI service repository uses, with thecheckgoal bound tovalidateso a missing header fails the build.Bump to 1.2.11. 1.2.10 is already published and immutable.
Note on the plugin's include list
The include list is deliberately explicit rather than pattern-based. Most of this tree is upstream MVEL and vendored ASM, whose headers must survive verbatim, and ThingsBoard-authored files sit in the same packages as upstream ones — no path pattern separates them. Listing the files keeps
formatfrom ever reaching upstream code: a new ThingsBoard file is merely unmanaged until it is added, rather than an upstream file being silently overwritten.skipExistingHeaderswould not work here either — 56 upstream MVEL files carry no header at all and would be stamped as ThingsBoard's.Please add new ThingsBoard-authored files to that list. There is a comment in
pom.xmlsaying so.Not addressed here
The pom still configures
central-publishing-maven-pluginandmaven-gpg-plugin, which is how 1.2.8 and 1.2.9 reached Maven Central, but 1.2.10 was published to the ThingsBoard Artifactory repository instead. Worth settling where 1.2.11 should go before cutting the release; I have not touched the deployment configuration.No source behaviour changes — the full suite (1369 tests) passes unchanged.