SteNicholas commented on code in PR #244:
URL: https://github.com/apache/paimon-cpp/pull/244#discussion_r3849088346
##########
cmake_modules/arrow.diff:
##########
@@ -15,6 +15,13 @@ diff --git a/cpp/cmake_modules/ThirdpartyToolchain.cmake
b/cpp/cmake_modules/Thi
index 8cb3ec83f5..0765df8fa8 100644
--- a/cpp/cmake_modules/ThirdpartyToolchain.cmake
+++ b/cpp/cmake_modules/ThirdpartyToolchain.cmake
+@@ -814,5 +814,6 @@ if(DEFINED ENV{ARROW_THRIFT_URL})
+ set(THRIFT_SOURCE_URL "$ENV{ARROW_THRIFT_URL}")
+ else()
+ set_urls(THRIFT_SOURCE_URL
++
"https://archive.apache.org/dist/thrift/${ARROW_THRIFT_BUILD_VERSION}/thrift-${ARROW_THRIFT_BUILD_VERSION}.tar.gz";
Review Comment:
**Scope: this looks like an unrelated change riding along.**
Commit `bb1f37d` ("build: use archives for retired Apache dependencies")
changes `cmake_modules/arrow.diff` and `cmake_modules/orc.diff`, which have
nothing to do with DLF authentication, and the PR description does not mention
it at all (it says "No storage-format or REST protocol changes" and stops
there). `AGENTS.md` asks to keep a change focused and to leave third-party
patches alone unless the task requires it.
Could this be split into its own PR? If it has to stay here because CI
cannot build without it, please at least say so in the description.
One small thing to double-check either way: inserting this hunk at line 814
leaves the *new-side* line numbers of the following pre-existing `@@ -983,6
+983,11 @@` hunk stale by one. `git apply` matches on content so this normally
still applies, but it is worth confirming once in a clean build that the Arrow
patch is still accepted.
--
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]
