Skip to content

Conversation

@LiHua000
Copy link
Contributor

Log: as title

Log: as title
@deepin-ci-robot
Copy link

deepin pr auto review

我来帮你分析这个代码变更:

  1. CMakeLists.txt 的变更:
-pkg_check_modules(PDFIUM REQUIRED deepin-pdfium${PDF_LIB_VERSION})

移除了对 PDFIUM 的依赖检查。这个改动需要谨慎评估:

  • 如果项目不再需要 PDF 支持,这个移除是合理的
  • 如果仍然需要 PDF 功能,这个移除会导致编译或运行时问题
  • 建议确认是否真的不再需要 PDF 支持,或者是否有其他方式引入 PDF 依赖
  1. XpsTextExtractor.cpp 的变更:
-        
+

这是一个纯空白字符的变更,移除了一行空行。这种变更:

  • 对代码功能没有影响
  • 不建议单独提交这样的格式变更
  • 如果要做格式调整,建议使用代码格式化工具统一处理,避免手动修改

改进建议:

  1. 对于 CMakeLists.txt:
  • 建议添加注释说明移除 PDFIUM 依赖的原因
  • 如果是暂时移除,建议添加 TODO 标记说明后续计划
  • 考虑添加条件编译选项,让 PDF 支持可配置
  1. 对于 XpsTextExtractor.cpp:
  • 建议撤销这个空白字符的变更
  • 如果要做代码格式化,建议:
    • 使用统一的代码格式化工具(如 clang-format)
    • 在一个单独的提交中处理所有格式变更
    • 在项目中添加格式化配置文件,确保一致性
  1. 通用建议:
  • 代码变更应该有明确的 functional 改进或 bug 修复
  • 纯格式变更应该与功能变更分开提交
  • 重要的依赖变更(如 PDFIUM)应该有充分的文档说明
  • 建议在提交信息中详细说明变更的原因和影响

@deepin-ci-robot
Copy link

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: LiHua000, lzwind

The full list of commands accepted by this bot can be found here.

Details Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@LiHua000
Copy link
Contributor Author

/merge

@deepin-bot deepin-bot bot merged commit 1867d2c into linuxdeepin:master Nov 25, 2025
6 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.

3 participants