kevinjqliu commented on code in PR #3562:
URL: https://github.com/apache/iceberg-rust/pull/3562#discussion_r4396709325
##########
bindings/python/about.toml:
##########
@@ -65,17 +65,3 @@ license = "MIT"
[[libm.clarify.files]]
checksum = "3823dda7cf046602f4b4e77ec8e227863dc4736037cc85bb33d9f19febe16bb7"
path = "LICENSE.txt"
-
-# zstd-sys's LICENSE covers only its Rust code. Without this, cargo-about omits
Review Comment:
trimming this, no longer needed. it was previously added because my agent
was using an outdated version of cargo-about, now fixed in #3561
##########
.github/workflows/bindings_python_ci.yml:
##########
Review Comment:
`.github/actions/generate-python-license/action.yml` needs the rust
toolchain, so dont skip toolchain setup
##########
.github/actions/generate-python-license/action.yml:
##########
@@ -30,7 +30,12 @@ runs:
shell: bash
working-directory: bindings/python
run: |
- cargo about generate -c about.toml --locked --fail --manifest-path
Cargo.toml about.hbs > cargo-about-output.txt
+ # --fail misses warnings and license template fallbacks.
+ cargo about generate -c about.toml --locked --fail --manifest-path
Cargo.toml -o cargo-about-output.txt about.hbs 2>&1 | tee
"$RUNNER_TEMP/cargo-about.log"
+ if [[ -s "$RUNNER_TEMP/cargo-about.log" ]] || grep -n '<year>'
cargo-about-output.txt; then
+ echo "::error::bindings/python/about.toml needs an update, see
bindings/python/LICENSING.md"
+ exit 1
+ fi
Review Comment:
add a check for warn logs and <year> output
##########
bindings/python/LICENSING.md:
##########
@@ -60,12 +60,14 @@ Note that in the repository, `bindings/python/LICENSE` and
`NOTICE` are symlinks
## Keeping entries up to date
-Pull request CI runs the action, so it fails when generation fails, for
example on a license that isn't in `accepted`.
-
-Everything else needs a manual check and update before every release: the
clarifications and `targets` in `about.toml`, `third-party-licenses.txt` and
`third-party-notices.txt`. They don't change with the dependencies.
-
-Run this from `bindings/python` to check. A `WARN` or `warning:` on stderr
means a clarification no longer applies, and `<year>` in the output means no
license file was matched:
+Pull request CI checks `about.toml`. It fails on a license that isn't in
`accepted`, on a `cargo about` warning such as a stale clarification, and on
`<year>` in the output, which means no license file was found. To reproduce a
failure, run this from `bindings/python`:
```shell
-make -s cargo-about | grep -nE '<year>|<owner>|<copyright holders>'
+make -s cargo-about | grep -n '<year>'
```
+
+CI doesn't check the other entries. Before each release, from
`bindings/python`, check that:
+
+- `targets` in `about.toml` matches the wheel targets in `release_python.yml`
and `release_python_nightly.yml`.
+- Each crate in `third-party-licenses.txt` is in the `make -s cargo-about`
output at the version the entry names. If the version changed, re-check the
entry against the new version. If the crate is gone, remove the entry.
+- `third-party-notices.txt` matches the `NOTICE` files of the dependencies,
found with the command in [Relevant files](#relevant-files).
Review Comment:
i like this part. its very explicit
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
