Repository navigation
chore: expand NOTICE attribution and bundle it in base artifact - #2593
Conversation
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This is a focused, clean chore PR that adds a NOTICE file for Apache 2.0 §4(d) attribution propagation and copies it into the base-template build artifact alongside LICENSE. The scope is narrow and the implementation is correct. Two minor nits are noted below, neither of which blocks merge.
Correctness
The change does exactly what the title claims. NOTICE is added at the repo root and prepare-base.sh copies it into build-wheels-base/ adjacent to LICENSE. No logic errors.
One very minor observation: the PR body cites §4(c) as the motivating clause, but §4(c) is the broader "retain all copyright and attribution notices" requirement. The prepare-base.sh code comment — added by this PR — correctly cites §4(d), which is the clause specifically about propagating NOTICE files in Derivative Works:
# Copy Apache License and NOTICE attribution file. Apache 2.0 §4(d) requires
# downstream redistributors to propagate NOTICE, so both must ship with the
# base template artifact.
The code comment is accurate. The PR description's §4(c) reference is slightly imprecise but has no practical consequence.
Docs
The PR body explicitly acknowledges that prepare-core.sh, prepare-cli.sh, and prepare-starterApp.sh do not currently bundle LICENSE either, and defers fixing them as a follow-up. That is a reasonable scope decision, but it does mean those artifacts ship without either file. If the intent is full Apache 2.0 compliance across all build artifacts, a follow-up issue or TODO comment in those scripts would help ensure this doesn't get lost.
cli/LICENSE note: NOTICE at line 14 states that Tom King's CLI was "integrated into the monorepo under Apache License 2.0 with attribution preserved in cli/LICENSE." The actual cli/LICENSE file still contains the original MIT License text verbatim with no Apache 2.0 header or cross-reference. This is consistent with the PR's stated intent (preserve attribution, not relicense), but a reader comparing cli/LICENSE to the NOTICE claim may find the framing slightly confusing — cli/LICENSE remains MIT, not a hybrid. A one-line comment at the top of cli/LICENSE (e.g. # This file preserves the original MIT license of the Wheels CLI for attribution purposes.) would clarify intent. Not a blocker.
Commits
Single commit: chore: add NOTICE file for Apache 2.0 attribution (#2573) — valid type, clean subject well under 100 chars, DCO sign-off present (Signed-off-by: Peter Amiri <peter@alurium.com>). Conforms to commitlint.config.js.
The old comment # Copy Apache License how are you? in prepare-base.sh (an apparent AI-generated artifact) is cleaned up as a side-effect — welcome.
d8d5833 to
a2a7bbf
Compare
Wheels Bot -- Reviewer B (round 1)A's review is accurate and proportionate for this diff. No sycophancy, no false positives, no missed issues. The two observations A raised are both correct and correctly weighted as non-blocking. SycophancyNone detected. The TL;DR is supported by actual analysis rather than empty praise, and A withheld APPROVED rather than rubber-stamping a chore PR. False positivesNone detected.
Missed issuesNone detected. The diff is two files:
The scope decisions in the PR description (deferring other build scripts, not modifying LICENSE) are reasonable and A noted them correctly. Verdict alignmentA used COMMENTED rather than APPROVED while explicitly stating "neither of which blocks merge." That is a mild inconsistency in form, but the substance of the recommendation is clear: the PR is merge-ready. No realignment needed. ConvergenceAligned. A's analysis is accurate, the diff is clean, and there is nothing actionable remaining. Joint recommendation: approve and merge. |
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This PR adds a NOTICE file for Apache 2.0 §4(d) compliance and bundles it alongside LICENSE in the base-template build artifact. The implementation is correct and the scope is intentionally tight. Two minor nits are noted; neither blocks merge. Verdict: comment (no changes required).
Correctness
The change does exactly what the PR claims. NOTICE is added at the repo root, and prepare-base.sh copies it into build-wheels-base/ next to LICENSE. No logic errors. The shell script uses set -e and a plain cp, which is the right idiom here.
The code comment in prepare-base.sh (lines 48–50) correctly cites §4(d) — the clause specifically requiring NOTICE propagation in Derivative Works. The commit body and the PR description both reference §4(c) instead. §4(c) covers copyright and attribution notices more broadly; §4(d) is the NOTICE-specific clause. The code is right; the prose is slightly imprecise. Not a correctness defect, but worth a tidy if the commit is squash-edited before merge.
Docs
prepare-core.sh, prepare-cli.sh, and prepare-starterApp.sh do not currently copy LICENSE either — the PR body explicitly defers those as a follow-up. That is a reasonable scope decision. A follow-up issue would help ensure the gap does not get lost.
The NOTICE text at line 22 links to:
http://www.apache.org/licenses/LICENSE-2.0
This HTTP URL is canonical in the Apache 2.0 template text (the ASF intentionally uses HTTP for that URL for historical/redirect reasons), so this is correct as-is.
Commits
Single commit: chore: add NOTICE file for Apache 2.0 attribution (#2573) — valid type, clean subject well under 100 chars, DCO sign-off present (Signed-off-by: Peter Amiri <peter@alurium.com>). Conforms to commitlint.config.js.
Wheels Bot — Reviewer B (round 1)A's review is accurate and proportionate for this diff. The two nits A raised (§4(c) vs §4(d) in the prose, and the deferred follow-up for other build scripts) are correct and correctly weighted as non-blocking. No issues to dispute or escalate. Joint recommendation: approve and merge. SycophancyNone detected. A used COMMENTED rather than APPROVED, and the TL;DR is grounded in actual diff analysis rather than empty praise. False positivesNone detected.
Missed issuesNone detected. The diff is two files: a new 30-line plain-text attribution file and a two-line shell script change (removes an AI artifact, adds One bot-infrastructure note (not a PR issue): A's review marker contains SHA Verdict alignmentA's COMMENTED verdict with "no changes required" and "neither blocks merge" is substantively correct. COMMENTED is a slightly odd form choice when no changes are needed (APPROVED would be more precise), but this does not misrepresent the recommendation. ConvergenceAligned. A's analysis is accurate, the diff is clean, and there is nothing actionable remaining. Joint recommendation: approve and merge. |
a2a7bbf to
5d14da3
Compare
There was a problem hiding this comment.
Wheels Bot — Reviewer A
TL;DR: This PR expands the NOTICE file with broader contributor attribution (original CFWheels authors, named long-running maintainers) and bundles it into the base-template build artifact so Apache 2.0 §4(d) propagation actually happens for ForgeBox installs. The implementation is correct and the commit is well-formed. Two minor nits below, neither blocking. Verdict: comment.
Correctness
The change does exactly what the PR claims. The two-file diff is clean:
NOTICEgains a copyright header, contributor attribution block, separator lines, a section title for the existing MIT relicensing clause, and a trailing Apache 2.0 license notice.prepare-base.shaddscp NOTICE "${BUILD_DIR}/"alongside the existingcp LICENSEunder a corrected comment. The old# Copy Apache License how are you?artifact from PR #2594 is cleaned up as a side effect.
No logic errors. set -e is already in effect; the plain cp is the right idiom.
Nit 1 — section 4(c) vs 4(d) in prose. The PR body and commit body both open with:
Apache License 2.0 section 4(c) provides for a NOTICE file to carry attribution that must propagate to downstream redistributions.
Section 4(c) covers retention of copyright and attribution notices broadly. Section 4(d) is the clause specifically requiring NOTICE propagation in Derivative Works. The code comment in prepare-base.sh line 48 is correct (# Apache 2.0 section 4(d) requires NOTICE to propagate to derivatives.). The prose mislabels the clause. This was flagged in earlier review rounds on prior SHAs and is still present here. It has no practical legal consequence but is worth correcting if the commit is edited before merge.
Nit 2 — Apache license boilerplate at the bottom of NOTICE. The new NOTICE closes with the standard "Licensed under the Apache License, Version 2.0 / Unless required by applicable law..." boilerplate. NOTICE files are purpose-built for attribution notices, not for restating license terms (that lives in LICENSE). The canonical ASF NOTICE template omits this boilerplate entirely. Including it is not wrong, and does not duplicate the verbatim LICENSE text the PR carefully preserves, so license-detection fingerprinting should be unaffected. But it is non-standard and may confuse readers encountering the file in isolation. If the intent is self-containment for end-users finding NOTICE inside a ForgeBox artifact, a single line — This product is licensed under the Apache License, Version 2.0 (see LICENSE). — would be clearer and less likely to be mistaken for a standalone license text.
Commits
Single commit: chore: expand NOTICE attribution and bundle it in base artifact (#2573)
- Type
chore: valid percommitlint.config.js. - Subject: 60 chars, not ALL-CAPS, no forbidden patterns.
- Body explains the why (propagation requirements, scope decisions re: other scripts, LICENSE fingerprint preservation): well-written.
- DCO sign-off present:
Signed-off-by: Peter Amiri <peter@alurium.com>.
Scope gap (acknowledged, not a blocker)
prepare-core.sh, prepare-cli.sh, and prepare-starterApp.sh still copy neither LICENSE nor NOTICE. The PR body explicitly defers this as a follow-up. A tracking issue would help ensure it is not lost.
Wheels Bot — Reviewer B (round 1)A's review is accurate and proportionate for this diff. Both nits are real, correctly weighted as non-blocking, and grounded in the actual diff content. No sycophancy, no false positives, no missed issues. Joint recommendation: approve and merge. SycophancyNone detected. A withheld APPROVED and issued a substantive analysis rather than rubber-stamping a chore PR. The TL;DR is supported by specific diff observations. False positivesNone detected.
Missed issuesNone detected. The diff is exactly two files: a plain-text attribution update and a two-line shell script change (remove AI artifact comment, add Verdict alignmentA's COMMENTED verdict with explicit "neither blocks merge" is substantively "approve." The form is slightly inconsistent — APPROVED would match the substance — but the recommendation is unambiguous. No realignment needed. ConvergenceAligned. A's analysis is accurate, both nits are real but non-blocking, and there is nothing actionable remaining. Joint recommendation: approve and merge. |
Apache License 2.0 §4(d) requires NOTICE attribution to propagate to derivative works. PR #2594 added an initial NOTICE focused on the cli/LICENSE MIT→Apache reconciliation; this change extends the same file with the broader contributor attribution called for in #2573 — original CFWheels authors (Per Djurner et al.), the named long-running contributors, and a pointer to git log for the full list — while preserving the Tom King / wheels-cli MIT relicensing section as a dedicated subsection. Also updates tools/build/scripts/prepare-base.sh to bundle NOTICE alongside LICENSE in the base template build artifact, so the ForgeBox-published scaffold (and anything redistributing it) carries the attribution forward. Footer of NOTICE references LICENSE rather than restating the full Apache 2.0 boilerplate, matching the ASF canonical template convention (NOTICE is for attribution; license terms live in LICENSE). LICENSE itself is intentionally not modified; keeping the verbatim Apache 2.0 text intact preserves the fingerprint that license-detection tooling (FOSSA, ScanCode, GitHub linguist) relies on. Closes #2573. Signed-off-by: Peter Amiri <peter@alurium.com> Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
5d14da3 to
6c219f5
Compare
…cts (#2597) Apache 2.0 §4(a) requires LICENSE in every distributed artifact and §4(d) requires NOTICE to propagate to derivatives. PR #2593 covered the base-template artifact via prepare-base.sh but left the three sibling prepare scripts (core, cli, starter-app) bundling neither file. Mirror the pattern across all four release-artifact prepare scripts and pin the contract with a source-level regression guard. Fixes #2596 Signed-off-by: wheels-bot[bot] <wheels-bot[bot]@users.noreply.github.com> Co-authored-by: wheels-bot[bot] <wheels-bot[bot]@users.noreply.github.com> Co-authored-by: Peter Amiri <peter@alurium.com>
Summary
Closes #2573.
PR #2594 landed first with a NOTICE focused on the
cli/LICENSEMIT→Apache reconciliation. This PR extends the sameNOTICEwith the broader contributor attribution called for in #2573, and bundles the file into the release scaffold so Apache 2.0 §4(d) propagation actually happens.NOTICEwith attribution to original CFWheels contributors (Per Djurner et al.) and named long-running contributors (Andy Bellenie, Tony Petruzzi, Chris Peters) — preserves chore: relicense cli/LICENSE to Apache 2.0 + add NOTICE attribution #2594's Tom King / wheels-cli MIT relicensing section as a dedicated subsection.tools/build/scripts/prepare-base.shtocp NOTICE "${BUILD_DIR}/"alongside the existingcp LICENSE. Apache 2.0 §4(d) requires downstream redistributors to propagate NOTICE; bundling it in the base-template artifact is what makes that work in practice for ForgeBox installs and any scaffold-based redistribution.LICENSErather than restating the Apache 2.0 boilerplate (Reviewer A nit) — matches the ASF canonical NOTICE template convention.Why this matters
Apache License 2.0 §4(d) requires that when a Work includes a
NOTICEfile, any Derivative Works that get distributed must include a readable copy of the attribution notices it contains. Until #2594 landed, the repo had none; this PR finishes the job by addressing the broader contributor attribution that #2594's narrower scope deliberately deferred.Scope decisions
LICENSEintentionally not modified. The existingLICENSEis verbatim Apache 2.0 text. License-detection tooling (FOSSA, ScanCode, GitHub linguist) fingerprints that exact body — adding a footer or header risks false negatives in compliance scans. The issue notes the cross-reference is optional ("Apache 2.0 doesn't require it"); skipping it preserves detection accuracy with no real legal cost.NOTICEships in those automatically — no workflow change needed.LICENSEeither (they declarelicense: Apache-2.0as metadata only). That's a separate compliance pass; this PR keeps scope tight to the monorepo'sNOTICEstory plus the base-template artifact.prepare-core.sh/prepare-cli.sh/prepare-starterApp.shdon't currently copyLICENSEeither — leaving them untouched matches the existing convention. Reviewer A flagged this as a scope gap worth tracking; will open a follow-up issue after merge.Test plan
Build & Publish Releaseworkflow ondeveloprunsprepare-base.shand validates the base-template artifact — confirmNOTICElands inbuild-wheels-base/next toLICENSE.NOTICEon the PR diff and confirm formatting renders correctly and both attribution sections coexist.🤖 Generated with Claude Code