Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

协作开发 CodeReview CheckList #857

Closed
wklken opened this issue Dec 16, 2022 · 3 comments
Closed

协作开发 CodeReview CheckList #857

wklken opened this issue Dec 16, 2022 · 3 comments
Assignees
Labels

Comments

@wklken
Copy link
Collaborator

wklken commented Dec 16, 2022

目标: 减少 CR 时间, 提升协作效率, 尽量在前期规避掉命名/风格/规范/冲突/扫描等初级问题

开发前

  • 需要从 PR 目标分支切出来改
  • 一个需求一个 PR, 禁止多个需求放在一个 PR 中一起提交

必须符合一些基础的规范

  • 除了logger, 其他字符串拼装尽量用 f-string
  • 所有函数/参数, 尽量要使用 typehint
  • 常量/方法名/ErrorCode 名称最好多人review确认(这块每次都会有)
  • 一致性: 一个概念只有一个确定的命名, 不要有多个; 不同概念的命名必须不同
  • 单一职责原则, 一个函数只做一件事情
  • 函数的返回值必须一致, 要么都是bool要么都raise; 所有return的参数必须一致
  • URL 需要检查是否符合restful风格
  • DRF的输入/输出都必须有 SLZ, 禁止直接构造dict放入response
  • log打印要包含尽量都得上下文, 看到这个日志能定位问题(反例: 只打个error, 或者data, 没有上下文/不知道是谁)
  • except代码块中打日志用 logger.exception
  • 不能吞掉异常 except: pass
  • 禁止: 在代码中使用print打印日志
  • 禁止: 日志打印中禁止打印敏感信息, 例如密码/解密的明文等信息

提交前

  • 需要经过严格自测, 再小的变更也得验证(防止类似语法错误等低级问题进入下一阶段)
  • 提交 PR的开发者需要签名 CLA
  • 新增文件头需要带license, 并且是本项目的license头
  • 必须配置pre-commit, 必须本地检查都通过才能提交(flake8/isort/black/mypy)

提交 MR 后

  • 提交后: 有冲突需要修复
  • 提交后: github action需要检查通过, 风格检查+单测
  • 提交后: 腾讯内部的codecc扫描失败需要确认, 大概率是复杂度超过最大限制, 需要重构拆解函数

第一轮, 导师Review

按照需求/上面的checklist进行review, 可能review多次, 直到符合导师的 Review 要求
全部修复确认无误后, 进入下一轮

第二轮, 项目/模块负责人Review


其他:

  • 每次 Review 后的修复需要关闭对应的conversation, 如果有不处理的需要说明原因
  • 需要有单测(当前未要求, 未来会要求)

相关链接:

  • 蓝鲸最佳实践
  • 书籍: <<编写可读代码的艺术>>
  • 书籍: <<编写高质量代码—改善Python程序的91个建议>>
@wklken wklken added the Type: proposal Proposal label Dec 19, 2022
@Canway-shiisa Canway-shiisa self-assigned this Jan 3, 2023
@wklken
Copy link
Collaborator Author

wklken commented Jan 9, 2023

需要继续补充历史 PR 中可以抽出规范的内容:

  1. 用户管理
  2. 权限中心

@wklken
Copy link
Collaborator Author

wklken commented Jan 12, 2023

待梳理

from #565

  • 配置使用项目中的pre-commint: black/pep8/isort/mypy检查
  • 使用 f-string 而不是% or .format(日志打印除外)
  • 使用 typehint
  • 单元测试
  • 必须搭建本地环境,PR需要在本地进行测试OK后才能提交

协作开发:

  • 导师制, 这边只review导师确认过的 PR(即, 需要内部完成review)
  • 个人开发, 即使变更再少, 也需要经过严格自测(之前PR 中出现过语法错误/变量不存在等非常低级的错误)

规则:

  • 每次review不超过 3 轮, 3 轮没通过, close, 由导师重新梳理/变更/测试后提交 PR

@wklken
Copy link
Collaborator Author

wklken commented Feb 6, 2023

其他:

@wklken wklken closed this as completed Jun 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants