Repository navigation
chore(release): port the dependency license check from shell to Python - #3379
Conversation
There was a problem hiding this comment.
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Copilot review overview
3 open findings
dev/release/licenses.pyrequires Python 3.11+ fortomllib, but the… · Newcargo install --locked cargo-deny) while the repo pins a… · Newactions/setup-python, invokingpython(rather… · New
What changed in this PR
Migrates the Rust dependency license validation step from a Bash script to a Python CLI, updating developer docs, Make targets, and CI to use the new entrypoint.
Changes:
- Added
dev/release/licenses.pyimplementingdependencies checkviacargo deny check license - Updated docs/scripts/Makefile/CI to invoke the new Python command
- Removed the legacy
dev/release/dependencies.sh
| File | Description |
|---|---|
| website/src/release.md | Updates release instructions to use the new Python CLI. |
| dev/release/licenses.py | New Python CLI replacing the Bash license check logic. |
| dev/release/dependencies.sh | Removes the old Bash implementation. |
| dev/release/create_rc.sh | Routes RC creation license checks through the new Python CLI and requires python3. |
| dev/release/README.md | Updates local release docs to the new Python command and notes CI coverage. |
| Makefile | Swaps make target to run the Python-based license check. |
| .github/workflows/ci.yml | Installs Python and switches CI dependency license step to the Python CLI. |
🧠 Review effort: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
FYI, I opened the following PR to preview the full changes we're working towards: #3380 |
This change migrates from the old shell script for cargo-deny to a new Python script. This new script will contain logic for dealing with dependencies and their licenses. This commit simply translates the existing logic. In later changes, this script will be extended with: - Clarifications for existing dependencies where licenses are misreported, with additional verifications added in the script itself - Logic for generating the attribution documentation for the pyiceberg-core package.
ee1eed1 to
4a3c5f9
Compare
|
@CTTY or @kevinjqliu, would you be able to help push this one through? Thanks in advance! FYI, I have a draft of all the changes coming here: #3380 |
CTTY
left a comment
CTTY
left a comment
There was a problem hiding this comment.
Thanks Danny for working on this! we have one less loose end now :D
Which issue does this PR close?
Part of the solution for #3239.
What changes are included in this PR?
This PR migrates from the old shell script for cargo-deny to a new Python script.
This new script will contain logic for dealing with dependencies and their licenses. This commit simply translates the existing logic.
In later changes, this script will be extended with:
Are these changes tested?
No. I have done manual testing but I have not invested in automated testing here.
AI Disclosure
I used an LLM to generate the outline. It also suggested the additional checks after highlighting that errors were not reported by cargo-deny and cargo-about.
I have reviewed and stand by all changes.