背景
mention 系 reason(review_requested 等)の通知は @channel / @everyone でチャンネル全体に通知しているが、CI・自動チェックがまだ成功していない PR はレビューできる状態になっていないことが多く、その時点でメンションで急かす必要がない。
方針
挙動
- 対象 PR のチェックが成功していない場合、通知自体は送るがメンション(
@channel / @everyone)だけを外す
- 判定対象は
subject.type == "PullRequest" の通知のみ(Issue 等は従来どおり)
- チェックが 1 つも設定されていない PR は「未成功」扱いにせず、従来どおりメンションする(checks 未設定のリポジトリを巻き込まない)
- チェック状態の取得に失敗した場合は安全側に倒してメンションする(見逃しより誤メンションを許容)
reason による例外は設けない
当初は「mention / team_mention は人間による明示的な呼びかけなので CI 状態に関わらずメンションを維持する」方針だったが、これは実現できないため取り下げた(PR #107 レビュー指摘)。
reason は「そのスレッドを購読している理由」であってイベント種別ではないため、一度メンションされた PR はその後の push やコメントでも reason=mention のまま届く。これを例外にすると、最も関与している PR でこそチェックが赤いままチャンネル全体を叩くことになり、この issue の目的と正反対になる。
「今回の更新が実際にメンションだったか」を判別する案(コメント本文の @login を見る等)も検討したが、自分の login を得る追加 API が必要・team_mention(@org/team)は判定不可・今回取得したコメント以外のメンションを拾えない、という制約があるため見送った。
メンションされた通知自体は従来どおり届き、@channel が付かなくなるだけなので見逃しにはならない。
判定方法
subject.url(/repos/:owner/:repo/pulls/:number)から PR を取得して head SHA を得る
GET /repos/:owner/:repo/commits/:ref/check-runs と combined status(/commits/:ref/status)で成否を判定し、失敗・pending が 1 つでもあればメンションを抑止する
check runs と commit status は別系統で片方にしか結果が出ないことがあるため、両方を取得して厳しい方(Failure > Pending > Success > チェック無し)を採る。commit status は 0 件でも state: "pending" が返るため、total_count で「チェック無し」を先に判別する。
実装箇所
src/github/models.cr: ChecksState と、判定対象かを返す Notification#checks_gated? を追加
src/github/repository.cr: PR 取得・チェック状態取得のメソッドを追加
src/github/usecase.cr: build_message で notify.mention? に加えチェック成否を反映して Notify::Message.mention を決める
spec/github/usecase_spec.cr / spec/github/models_spec.cr: 成功 / 失敗 / pending / チェックなし / 取得失敗 の各ケースを追加
コスト
抑止判定の対象となる PR 通知 1 件あたり +3 API リクエスト。毎分実行だが未読通知のみが対象なので許容範囲とする。
対応 PR
#107
背景
mention 系 reason(
review_requested等)の通知は@channel/@everyoneでチャンネル全体に通知しているが、CI・自動チェックがまだ成功していない PR はレビューできる状態になっていないことが多く、その時点でメンションで急かす必要がない。方針
挙動
@channel/@everyone)だけを外すsubject.type == "PullRequest"の通知のみ(Issue 等は従来どおり)reason による例外は設けない
当初は「
mention/team_mentionは人間による明示的な呼びかけなので CI 状態に関わらずメンションを維持する」方針だったが、これは実現できないため取り下げた(PR #107 レビュー指摘)。reasonは「そのスレッドを購読している理由」であってイベント種別ではないため、一度メンションされた PR はその後の push やコメントでもreason=mentionのまま届く。これを例外にすると、最も関与している PR でこそチェックが赤いままチャンネル全体を叩くことになり、この issue の目的と正反対になる。「今回の更新が実際にメンションだったか」を判別する案(コメント本文の
@loginを見る等)も検討したが、自分の login を得る追加 API が必要・team_mention(@org/team)は判定不可・今回取得したコメント以外のメンションを拾えない、という制約があるため見送った。メンションされた通知自体は従来どおり届き、
@channelが付かなくなるだけなので見逃しにはならない。判定方法
subject.url(/repos/:owner/:repo/pulls/:number)から PR を取得して head SHA を得るGET /repos/:owner/:repo/commits/:ref/check-runsと combined status(/commits/:ref/status)で成否を判定し、失敗・pending が 1 つでもあればメンションを抑止するcheck runs と commit status は別系統で片方にしか結果が出ないことがあるため、両方を取得して厳しい方(Failure > Pending > Success > チェック無し)を採る。commit status は 0 件でも
state: "pending"が返るため、total_countで「チェック無し」を先に判別する。実装箇所
src/github/models.cr:ChecksStateと、判定対象かを返すNotification#checks_gated?を追加src/github/repository.cr: PR 取得・チェック状態取得のメソッドを追加src/github/usecase.cr:build_messageでnotify.mention?に加えチェック成否を反映してNotify::Message.mentionを決めるspec/github/usecase_spec.cr/spec/github/models_spec.cr: 成功 / 失敗 / pending / チェックなし / 取得失敗 の各ケースを追加コスト
抑止判定の対象となる PR 通知 1 件あたり +3 API リクエスト。毎分実行だが未読通知のみが対象なので許容範囲とする。
対応 PR
#107