dannycjones commented on PR #3299:
URL: https://github.com/apache/iceberg-rust/pull/3299#issuecomment-5911321143

   > Thanks @dannycjones it mostly LGTM, couple of nits. Wondering how this has 
been tested?
   
   I've been testing by manually (with LLM) running this scenario, which 
includes:
   - a bad hash
   - a stale file entry
   - a stale clarification - crate is no longer used
   
   Applying this patch over the deny.toml.
   
   ```diff
   diff --git a/deny.toml b/deny.toml
   index 7f4b2a7c8..ad346cd13 100644
   --- a/deny.toml
   +++ b/deny.toml
   @@ -50,11 +50,13 @@ expression = "(MIT OR Apache-2.0) AND BSD-3-Clause AND 
(GPL-2.0-only OR BSD-3-Cl
    license-files = [
      { path = "LICENSE.Apache-2.0", hash = 0x7b466be4 },
      { path = "LICENSE.Mit", hash = 0xa237d234 },
   +  # A file that was removed upstream but is still attested here.
   +  { path = "LICENSE.Apache", hash = 0xbadbad04 },
      { path = "LICENSE.BSD-3-Clause", hash = 0xc9f5c4f6 },
      # BSD-3-Clause of vendored zstd src
      { path = "zstd/LICENSE", hash = 0x3bfe1fb1 },
      # GPL-2.0-only of vendored zstd src
   -  { path = "zstd/COPYING", hash = 0xeaa66bfd },
   +  { path = "zstd/COPYING", hash = 0xbadbad01 },
    ]
    
    # Mapped from OR to AND. The following PR should address upstream.
   @@ -63,7 +65,15 @@ license-files = [
    crate = "brotli-decompressor"
    expression = "BSD-3-Clause AND MIT"
    license-files = [
   -  { path = "LICENSE", hash = 0x1a60a92d }, # BSD-3-Clause
   +  { path = "LICENSE", hash = 0xbadbad02 }, # BSD-3-Clause
      # Missing: crate is MIT licensed but has no license text to refer to.
      # If the project drops the MIT licensing, we wouldn't know to update 
expression above.
    ]
   +
   +# A dependency that was dropped, but whose clarification was left behind.
   +[[licenses.clarify]]
   +crate = "openssl-sys"
   +expression = "Apache-2.0"
   +license-files = [
   +  { path = "LICENSE", hash = 0xbadbad03 },
   +]
   ```
   
   Basically checking that addressing each one gets us back to the toml that 
will be merged into main.
   
   I'm not super happy with it being manual tests but I don't think its worth 
investing more here right now.


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