Skip to content

refactor: remove third-party patchs and enforce submodule commit checks.#979

Merged
Kang-Meng merged 1 commit intojd-opensource:mainfrom
Kang-Meng:feat_submodule_init
Mar 4, 2026
Merged

refactor: remove third-party patchs and enforce submodule commit checks.#979
Kang-Meng merged 1 commit intojd-opensource:mainfrom
Kang-Meng:feat_submodule_init

Conversation

@Kang-Meng
Copy link
Collaborator

No description provided.

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors the third-party dependency initialization logic, making it more robust by checking whether initialization is necessary before running it. The changes are mostly in utils.py, with corresponding adjustments in setup.py. The new logic checks submodule status, patch application status, and the presence of required libraries. While the overall logic is a good improvement, there are some critical issues in the implementation where utility functions terminate the program using exit(1) instead of propagating errors to their callers. This makes the code less predictable, reusable, and testable. I've left comments with suggestions to fix these issues by replacing exit(1) with more appropriate error handling mechanisms.

@Kang-Meng Kang-Meng force-pushed the feat_submodule_init branch 2 times, most recently from fd3f75e to 902f1c1 Compare March 3, 2026 10:44
@Kang-Meng Kang-Meng changed the title refactor: centralize third-party initialization checks refactor: centralize third-party initialization checks. Mar 3, 2026
@Kang-Meng Kang-Meng force-pushed the feat_submodule_init branch 3 times, most recently from b07ac4d to e90e6f0 Compare March 3, 2026 16:19
@Kang-Meng Kang-Meng changed the title refactor: centralize third-party initialization checks. refactor: remove third-party patchs and enforce submodule commit checks. Mar 3, 2026
XuZhang99
XuZhang99 previously approved these changes Mar 4, 2026
yingxudeng
yingxudeng previously approved these changes Mar 4, 2026
@Kang-Meng Kang-Meng dismissed stale reviews from yingxudeng and XuZhang99 via 9391f72 March 4, 2026 07:51
@Kang-Meng Kang-Meng force-pushed the feat_submodule_init branch 2 times, most recently from 9391f72 to 7f64789 Compare March 4, 2026 07:53
@Kang-Meng Kang-Meng force-pushed the feat_submodule_init branch from 7f64789 to effa53f Compare March 4, 2026 08:44
Copy link
Collaborator

@yq33victor yq33victor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@Kang-Meng Kang-Meng merged commit 5137f4e into jd-opensource:main Mar 4, 2026
6 of 23 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants