kevinjqliu commented on code in PR #3293:
URL: https://github.com/apache/iceberg-rust/pull/3293#discussion_r4178260908


##########
dev/release/create_rc.sh:
##########
@@ -317,12 +307,7 @@ check_rc_tag_available() {
 }
 
 check_dependency_licenses() {
-  require_cargo_deny
-  (
-    trap - ERR
-    cd "${REPO_ROOT}"
-    cargo deny check license
-  )
+  "${SCRIPT_DIR}/dependencies.sh" check

Review Comment:
   use the same script for `create_rc.sh` 



##########
.github/workflows/ci.yml:
##########
@@ -59,7 +59,7 @@ jobs:
         # Keep versions in sync with the local install targets in Makefile.
         uses: taiki-e/install-action@d438492cf8a250514fa2d34b30bc3c0dc37c65ff 
# v2.87.8
         with:
-          tool: [email protected],[email protected]
+          tool: [email protected],[email protected],[email protected]

Review Comment:
   i think we might be able to get rid of the "keep versions in sync" pattern, 
now that we have `make install-*` in Makefile. 
   this can be a follow up 😄 



##########
deny.toml:
##########
@@ -26,7 +30,6 @@ allow = [
   "CC0-1.0",
   "Zlib",
   "CDLA-Permissive-2.0",
-  "bzip2-1.0.6",

Review Comment:
   this is flagged by my agent: removing `bzip2-1.0.6` since nothing uses it 
anymore. 
   
   cargo deny flags it as `license-not-encountered` even with `all-features`, 
and there's no bzip2 crate in `Cargo.lock`. looks like it was added back in the 
0.8.0 bump (#1938) for a dep we've since dropped. if it comes back, the check 
will fail and we can re-add it then.
   



##########
deny.toml:
##########
@@ -15,6 +15,15 @@
 # specific language governing permissions and limitations
 # under the License.
 
+# Dependency license policy. CI checks it on every pull request, and the
+# release scripts check it again before creating a release candidate. See
+# "Dependencies" in CONTRIBUTING.md for what to do when a license is rejected.
+
+[graph]
+# Also check dependencies that only optional features pull in, such as the
+# OpenDAL storage backends. Otherwise cargo-deny only follows default features.
+all-features = true

Review Comment:
   💯 ty for this! i didn't know about this option. 



##########
CONTRIBUTING.md:
##########
@@ -118,6 +118,14 @@ tested in CI and developers have reproducible builds.
 In `Cargo.toml`, we specify the minimum version required to use iceberg-rust. 
This allows users to choose their
 dependency versions without always upgrading to the latest.
 
+Dependency licenses must comply with the [ASF 3rd Party License 
Policy](https://www.apache.org/legal/resolved.html).
+CI checks them against `deny.toml` with `cargo deny`; run `make 
check-dependency-licenses` to check locally. If a
+license is rejected, find its category in the policy:

Review Comment:
   keeping this part concise



-- 
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]

Reply via email to