shuke987 commented on PR #12:
URL: https://github.com/apache/doris-skills/pull/12#issuecomment-5658439978

   @morningman 想确认下面两个场景,是否需要在这个 PR 中调整?基于当前提交 `f127757`。
   
   1. **只影响日志、措辞等的小回归,也必须定成 Major 吗?**
   
      
[文档中的规则](https://github.com/apache/doris-skills/blob/f127757db0171271c6d53b90311a2875d9443dcb/skills/doris-repo-review/references/doc-templates.md#L71-L75)允许可观测性、措辞、测试等类别的回归按实际影响定级,但[校验代码](https://github.com/apache/doris-skills/blob/f127757db0171271c6d53b90311a2875d9443dcb/skills/doris-repo-review/scripts/verify-review-docs.py#L197-L209)没有区分类别:只要标记为回归,就不能低于
 Major。
   
      例如,一个功能 PR 意外少输出了 debug 日志里的辅助信息,影响仅限排查便利性。审查者按文档写成“可观测性 / 回归:是 / 
Minor”,报告仍会被校验器拒绝。
   
      **这里期望的是“所有回归至少 Major”,还是“只有指定类别的回归至少 Major”?是否需要统一文档和代码?** 
前一种可以删除文档中的类别例外;后一种需要校验器识别类别。是否发布 PASS 仍可保留独立门槛。
   
   2. **PR 分支落后于主干时,会不会把主干的新修复误认为这个 PR 引入的回退?**
   
      
[新增回归判定](https://github.com/apache/doris-skills/blob/f127757db0171271c6d53b90311a2875d9443dcb/skills/doris-repo-review/SKILL.md#L372-L381)要求比较
 HEAD 和 `BASE_SHA`,但这里的 `BASE_SHA` 是目标分支的最新提交,PR diff 的起点则是两边的共同祖先 `MERGE_BASE`。
   
      例如:PR 分支创建后,主干新增了一处状态重置修复;PR 自己只改了一条日志。此时 PR 的 HEAD 
还没有那处主干修复,直接和最新主干比较,看起来像“少了一次重置”,按新增规则可能被判为 Major 回归。但它不是这个 PR 
删除的,正常合并也会保留主干的修复。
   
      **是否需要用 `MERGE_BASE` 判断“这个 PR 是否引入了回归”,或补充归因检查,排除仅因分支落后造成的差异?** 
对最新主干的兼容性检查可以另外进行。
   
   本地现有 47 项测试全部通过;另外验证了上述类别校验和分支分叉场景。想先确认这两处是否符合预期,再决定是否需要修改。
   


-- 
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]

Reply via email to