OpenProject 代码评审规范详解从拉取请求准备到合并落地的完整实践【免费下载链接】openprojectOpenProject is the leading open source project management software for product, project and portfolio management. A powerful Jira alternative with agile planning, issue tracking, roadmaps, Gantt charts, time tracking, collaboration features, and more. Available on premises or in the cloud. ⭐ Star us on GitHub项目地址: https://gitcode.com/GitHub_Trending/op/openproject本篇指南基于 OpenProject 官方的代码评审文档docs/development/code-review-guidelines/README.md系统讲解提交拉取请求前的风格检查、提交信息规范、测试与安全要求以及评审人视角下的接评审、正确性判断、沟通语言与合并策略。读完后你将能够按 OpenProject 的既有流程独立完成一次合规的 PR 提交与代码评审并理解 Lefthook 钩子、Rubocop、ESLint 等工具在评审链路中的实际作用。提交拉取请求前的准备编码风格让 Linter 成为你的第一评审人OpenProject 的编码风格遵循两条主线Ruby 代码遵循 Ruby 社区风格指南Ruby Style GuideJavaScript/TypeScript 前端代码遵循 AirBnB JavaScript 风格指南并叠加了针对 Angular 的扩展约定详见 docs/development/style-guide/frontend/。这两条规则不是停留在纸面上而是以可执行的工具配置固化在仓库中Rubocop 定义见 .rubocop.yml其中require了rubocop-openproject这一项目自定义插件并启用rubocop-rails、rubocop-rspec、rubocop-performance、rubocop-factory_bot、rubocop-capybara等插件AllCops目标 Ruby 版本为 3.4且NewCops: enable表示新版本 Rubocop 引入的 cops 默认生效见 .rubocop.yml#L1-L25。项目同时支持通过inherit_from引入.rubocop-local*.yml做个人化定制。前端 ESLint 配置位于 frontend/eslint.config.mjs编辑器中通常可直接集成 ESLint 检查与自动修复。YAML 文件尤其是config/locales/en.yml、config/locales/js-en.yml及各模块下的对应文件受 yamllint 配置 约束。文档对“历史债务”的态度很明确由于代码库年代较久存量代码未必全部符合风格指南但所有新代码必须遵守修改旧代码时顺手的风格改进是鼓励的但必须放在独立 commit 中避免风格改动淹没真实功能变更、干扰 diff 阅读。提交前请用 Rubocop 检查新代码可通过bin/dirty-rubocop做快速检查无需为存量代码的历史问题负责拉取请求在 CI 中还会被自动 lint。Lefthook提交时自动 lint 的完整配置官方推荐用 Lefthook 在git commit时自动执行检查安装方式为lefthook installlefthook 可通过brew、apt、bundle、npm等途径安装例如在Gemfile.local中加入 lefthook 使其进入 bundle。仓库根目录的 lefthook.yml 定义了具体钩子值得逐一了解pre-commit 阶段lefthook.yml#L14-L47针对暂存区文件并行执行钩子匹配文件执行的命令eslintfrontend/下的*.{js,ts,jsx,tsx}npx eslint {staged_files}rubocop*.rbbin/dirty-rubocop --uncommitted --force-exclusion {files}erb_lint*.erberb_lint {files}primer-view-components-version-is-the-same-everyhwereGemfile.lock、frontend/package.jsonscript/check_same_primer_view_components_version_everywhere校验 Ruby 与 JS 两侧的 Primer View Components 版本一致yamllint主翻译与模块翻译等 locale 文件yamllint {files}未安装时打印警告并跳过fix 阶段lefthook.yml#L59-L83是上述检查的自动修复版本需要显式触发例如# 修复暂存区文件 lefthook run fix # 修复相对 origin/dev 有变更的文件排除已删除的 lefthook run fix --files-from-stdin (git diff --name-only --diff-filterd origin/dev) # 修复仓库内全部文件 lefthook run fix --all-filesfix 阶段同样覆盖 eslint--fix、rubocop-a自动纠正、erb_lint--autocorrect并额外提供order_yaml命令script/order_yaml -i {files}来排序 locale YAML 键值与 yamllint 检查形成“检查—修复”闭环。配置文件的注释还说明若需在 Docker 容器上下文中改写执行命令可创建lefthook-local.yml覆盖对应配置。提交信息的结构第一行少于 72 个字符这是 GitHub 上标题开始出现 “…” 截断的阈值空一行变更的详细说明正文按 72 字符换行保证在git log中可读。这与 Git 官方书籍中关于分布式协作提交指南的建议一致。拉取请求描述的要求提供清晰的标题可选地链接到对应的 work package工作包附上评审、复现、准备测试数据所需的步骤说明在 OpenProject 社区中引用对应工单链接若没有工单需再次确认工单是否确实可省略。测试要求这是评审中最容易被逐项打回的部分文档给出了硬性清单所有 GitHub workflow 的 CI 动作必须全绿补充恰当的单元测试与集成测试用 rspec 测试应用逻辑用 feature 测试验证逻辑与 UI 的集成前端代码同样要用 feature spec 覆盖表单类变更不能只断言成功响应还要验证值确实被保存例如回读表单或查询数据库Bugfix必须包含一个能检测出该 bug 的测试防止回归翻译测试中严禁断言具体翻译字符串——翻译文案未来可能调整届时不应牵连测试修改关于偶发失败的“闪烁” spec代码库中已知存在少量随机失败的用例且正在修复中。若失败的代码并非你改动过的部分可以重试该 spec。仓库的测试布局与该要求对应模型与服务逻辑测试集中在 spec/models、spec/services行为级 UI 测试集中在 spec/features控制器与路由分别位于 spec/controllers 和 spec/routing。安全考量文档要求每位开发者与评审人都应通读 Rails 官方安全指南与 OWASP Top 10并将两者作为评审时的安全检查基线——即每次评审都默认以注入、访问控制、认证与会话等常见风险维度审视变更。变更日志不在源码中维护所有对 OpenProject 软件的变更都通过 OpenProject 社区的 work package 管理并记录非平凡变更或纯维护性变更如依赖版本升级必须关联一个工单在 PR 中以OP##Work package ID或工单 URL 的形式引用为避免不一致和重复劳动源码中不存在单独的 changelog发布说明来自公开可见工单的汇总见仓库内的 docs/release-notes/。标记代码为“可评审”状态在请求评审前先自查变更是否完整spec 补上了吗自己是否看过 diff是否遗忘了临时代码、调试语句PR 就绪后从 draft 转为正式 PR 并添加needs review标签。不要显式指派个人或群组作为评审人——除非你已与其协作过或有充分理由需要其特定反馈。等待评审人主动认领收到反馈或改动要求时不要个人化解读代码高度个人化每个人想法不同逐条回应反馈并解决已处理项处理完所有意见后向同一位评审人重新请求评审。执行评审时效性评审同事代码的优先级高于接手新工作。每天开工时或处理其他任务间隙应检查是否有等待认领的评审即needs review的 PR 列表。若某个评审长期无人认领可以请求群组评审或在开发者频道中挂出该 PR。认领评审准备评审某个 PR 时移除needs review标签——它是一个信号量semaphore确保同一时间只有一位开发者在评审该 PR若有关联工单且该工单有状态工作流将其置为in review并指派给自己可选把自己请求为 reviewer。正确性评审人的职责不是确认“这段代码是否和我会写的一样”——它不会一样——而是确认作者写出的代码是正确的。测试与评估代码时多考虑边界情况并复核测试覆盖但也无需对“合并后还是出了事”感到沮丧那是下一次避免同样失误的学习路径——而完全不做评审则不会带来任何进步。沟通语言评审内容是公开可见的因此保持友善与坦诚。具体建议用提问代替命令式要求存疑时先约会议澄清不要预设对方犯了错。测试与变更文档验证作者是否按上文要求补充了合适的测试测试功能或变更时至少按工单规格把快乐路径跑一遍能立即补充的小型文档变更随手补上破坏性变更如权限模型、面向开发者的接口要写进对应版本的 release notes 草稿若尚无草稿则新建一份。其他事项外部贡献者的 CLA检查作者是否已签署贡献者许可协议CLA未签署时礼貌地请求签署。仓库设有 GitHub workflow 自动校验贡献者是否签署了当前版本的 CLA该动作绿灯即视为此项完成。CLA 的背景说明见 docs/development/README.md 中的 “OpenProject Contributor License Agreement (CLA)” 一节。版权声明新增文件必须包含 OpenProject 版权声明可直接从仓库任意文件复制。仓库中 Ruby 文件的完整版权头范例可参考 Dangerfile#L1-L29——包含 GNU GPL v3 声明、ChiliProject/Redmine 的 fork 历史与 Copyright (C) the OpenProject GmbH 字样。添加 Gem更新 Gemfile 的同时必须同步更新Gemfile.lock仓库根目录的 Gemfile 与 Gemfile.lock 应成对提交。可读性评审的底线标准评审人应当不依赖代码之外的解释就能读懂代码。文档同时引用了两段值得记住的评审箴言直接说“Yup, looks good”从来不是错事。如果你总是刻意找茬唯一的结果是毁掉自己的可信度。代码评审不应草率——但也要及时同事在等你。完成评审与合并策略评审结束、已留下反馈时发布评审若有把关联 work package 指回开发者并置为in development。无需修改时批准approve该 PR使用 “Merge pull request” 按钮合并若有关联工单将其置为 merged实现类工单则为 closed并解除自己的指派。为什么用 merge 而不是 squashOpenProject 不使用默认 squash以保留原作者的提交信息——其中可能包含有价值的上下文。若 PR 中存在大量琐碎提交会先在 PR 内压缩这些提交再执行 merge对一个变更拥有完整信息被认为比严格线性的历史更重要。唯一例外是单 commit 的 PR可以直接用 “Rebase and merge” 应用到 dev 分支。值得一提的是仓库还通过 Danger 自动化了部分 PR 层面的检查根目录的 Dangerfile 依次引入了用户引用、发布迁移、project_id 契约、Primer 变更、迁移索引命名等检查逻辑.github/dangerfiles/下各自的 Dangerfile这些自动化检查与人工评审配合构成了完整的 PR 质量关口。延伸阅读前端开发风格与架构模式AirBnB 风格、Angular 风格、不可变数据流、单向数据流、容器/展示组件划分docs/development/style-guide/frontend/README.mdGit 工作流说明docs/development/git-workflow/Lefthook 钩子配置全文lefthook.ymlRubocop 项目配置.rubocop.yml【免费下载链接】openprojectOpenProject is the leading open source project management software for product, project and portfolio management. A powerful Jira alternative with agile planning, issue tracking, roadmaps, Gantt charts, time tracking, collaboration features, and more. Available on premises or in the cloud. ⭐ Star us on GitHub项目地址: https://gitcode.com/GitHub_Trending/op/openproject创作声明：本文部分内容由AI辅助生成（AIGC），仅供参考
