gemini-code-assist[bot] commented on code in PR #18779: URL: https://github.com/apache/tvm/pull/18779#discussion_r2808218833
########## tests/lint/cpplint.sh: ########## @@ -18,14 +18,9 @@ set -e -echo "Running 2 cpplints..." -python3 3rdparty/dmlc-core/scripts/lint.py --quiet tvm cpp \ - include src \ - examples/extension/src examples/graph_executor/src \ - tests/cpp tests/crt \ - --exclude_path "src/runtime/hexagon/rpc/hexagon_rpc.h" \ - "src/runtime/hexagon/rpc/hexagon_rpc_skel.c" \ - "src/runtime/hexagon/rpc/hexagon_rpc_stub.c" \ +echo "Running cpplint..." +# TODO: replace dmlc-core lint script with cpplint +echo "cpplint skipped (dmlc-core removed)" Review Comment:  This change disables the `cpplint` check. While removing the dependency on the `dmlc-core` lint script is correct, the check should be replaced with a direct call to `cpplint` as the TODO comment suggests, rather than being skipped. Disabling lint checks, even temporarily, can compromise code quality. ########## tests/lint/jnilint.sh: ########## @@ -18,4 +18,5 @@ set -e -python3 3rdparty/dmlc-core/scripts/lint.py tvm4j-jni cpp jvm/native/src +# TODO: replace dmlc-core lint script with cpplint +echo "jnilint skipped (dmlc-core removed)" Review Comment:  Similar to `cpplint.sh`, this change disables the `jnilint` check. The check should be reimplemented using `cpplint` directly instead of being skipped to maintain code quality for JNI sources. ########## apps/cpp_rpc/rpc_server.cc: ########## @@ -390,8 +390,6 @@ void ServerLoopFromChild(SOCKET socket) { void RPCServerCreate(std::string host, int port, int port_end, std::string tracker_addr, std::string key, std::string custom_addr, std::string work_dir, bool silent) { if (silent) { - // Only errors and fatal is logged - dmlc::InitLogging("--minloglevel=2"); } Review Comment:  This `if (silent)` block is now empty after removing `dmlc::InitLogging`. It should be removed to improve code clarity. The `silent` parameter is still passed to the `RPCServer` constructor, so it's being used elsewhere. ########## src/runtime/logging.cc: ########## @@ -274,8 +274,8 @@ std::string Backtrace() { namespace tvm { namespace runtime { -// Fallback to the dmlc implementation when backtrace is not available. -std::string Backtrace() { return dmlc::StackTrace(); } +// Fallback when libbacktrace is not available. +std::string Backtrace() { return ""; } Review Comment:  This change removes the fallback stack trace implementation that was provided by `dmlc::StackTrace()`. Now, on platforms without `libbacktrace` (like Windows or macOS without it explicitly installed), `Backtrace()` will return an empty string. This is a regression in debugging capability. Was this intentional? If not, a new fallback implementation for stack traces should be considered. -- 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]
