gemini-code-assist[bot] commented on code in PR #231: URL: https://github.com/apache/tvm-ffi/pull/231#discussion_r2500992496
########## python/tvm_ffi/utils/_build_optional_torch_c_dlpack.py: ########## @@ -567,6 +568,20 @@ } """ +def parse_env_flags(env_var_name) -> List[str]: Review Comment:  For better code clarity and to adhere to modern Python practices, it's best to provide a type hint for the `env_var_name` parameter. Based on its usage, it should be a string. ```suggestion def parse_env_flags(env_var_name: str) -> List[str]: ``` ########## python/tvm_ffi/utils/_build_optional_torch_c_dlpack.py: ########## @@ -791,6 +806,14 @@ def main() -> None: # noqa: PLR0912, PLR0915 ldflags.append(f"-L{python_libdir}") ldflags.append(f"-l{py_version}") + env_ldflags = parse_env_flags("DLPACK_EXTRA_LDFLAGS") + if env_ldflags is not None: + ldflags.extend(env_ldflags) + + env_cflags = parse_env_flags("DLPACK_EXTRA_CFLAGS") + if env_cflags is not None: + cflags.extend(env_cflags) Review Comment:  The `parse_env_flags` function is guaranteed to return a list, which will never be `None`. Therefore, the `if env_... is not None:` checks are redundant and can be removed to make the code more concise. ```python ldflags.extend(parse_env_flags("DLPACK_EXTRA_LDFLAGS")) cflags.extend(parse_env_flags("DLPACK_EXTRA_CFLAGS")) ``` -- 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]
