From 199b56e1c013758c538604c6abcefa59e0e9cb3d Mon Sep 17 00:00:00 2001 From: ma91n Date: Thu, 30 Jan 2025 10:55:17 +0900 Subject: [PATCH] ADD: auto number --- .vitepress/theme/style.css | 11 ++++ documents/forCodeReview/code_review.md | 80 +++++++++++++------------- 2 files changed, 51 insertions(+), 40 deletions(-) diff --git a/.vitepress/theme/style.css b/.vitepress/theme/style.css index 042e354..8a34e47 100644 --- a/.vitepress/theme/style.css +++ b/.vitepress/theme/style.css @@ -131,3 +131,14 @@ h5, h6 { page-break-after: avoid; } + +/* h2 タグにカウンターリセットを追加 */ +h2 { + counter-reset: section; +} + +/* h3 タグに連番を振る */ +h3::before { + counter-increment: section; + content: counter(section) ". "; +} diff --git a/documents/forCodeReview/code_review.md b/documents/forCodeReview/code_review.md index 70c72c5..4543f70 100644 --- a/documents/forCodeReview/code_review.md +++ b/documents/forCodeReview/code_review.md @@ -164,7 +164,7 @@ SQL: # レビュイーの推奨行動 -## 1. 重要な変更の場合、方向性を事前にチームと合意形成しておく +## 重要な変更の場合、方向性を事前にチームと合意形成しておく 実装する内容が、設計書に基づく新規機能の改修や、不具合改修でチケットに原因分析や対応方針が明記されている場合は、チーム内で改めて対応方針のすり合わせを行う必要はない。 @@ -188,7 +188,7 @@ SQL: 参考: [Design Doc でチームを跨いだ開発を円滑に行う \- 一休.com Developers Blog](https://user-first.ikyu.co.jp/entry/2024/12/09/143648) -## 2. プルリクエストの単位を小さくする +## プルリクエストの単位を小さくする 複数の改修内容が単一のプルリクエストに含まれており、変更内容が多い（20ファイル以上などの）場合、レビュアーの負荷が高くなってしまう。コミットを意味のある粒度に保ちつつ細かくするか、プルリクエストの粒度を細かくするかの2通りの対応が考えられる。 @@ -202,7 +202,7 @@ SQL: プルリクエストの粒度を細かくすることで、レビュアーの負荷を下げることができる。例えば、ある機能改修を行う際に、リファクタリングを行ってから作業をおこないたい場合は、リファクタリングのみを行うプルリクエストと、機能追加を行うプルリクエストを分けることがあげられる。なお、ローカル変数の名称を変更するといった小さな変更は、わざわざプルリクエストを分ける必要はない。 ::: -## 3. コミットメッセージはこだわらなくても良い +## コミットメッセージはこだわらなくても良い [commitlint](https://commitlint.js.org/) を適用し、レビュー負荷軽減のためコミットメッセージの統制を図るという考え方も存在する。 @@ -217,7 +217,7 @@ SQL: - プルリクエストの単位を小さくする前提であるため、コミットの粒度やメッセージを気にすることによるメリットが小さい - Gitブランチ規約から、最終的にスカッシュマージするため重要度が下がる -## 4. プルリクエストのタイトルにプレフィックスを入れる +## プルリクエストのタイトルにプレフィックスを入れる プルリクエストのタイトルに命名規則をもたせることで、レビュアーの負荷軽減に繋げることができる。また、履歴からのトラッキングが容易になる。 @@ -240,7 +240,7 @@ SQL: 関連: [Gitブランチフロー規約 \> ラベル](https://future-architect.github.io/coding-standards/documents/forGitBranch/git_branch_standards.html#%E3%83%A9%E3%83%98%E3%82%99%E3%83%AB%E8%A6%8F%E5%89%87) ::: -## 5. 差分（diff）を極小化することにこだわりすぎない +## 差分（diff）を極小化することにこだわりすぎない レビュー負荷軽減や、影響度の極小化の観点から、なるべく差分を小さくすることを志向した改修を行うべきか迷う場面がある。 @@ -251,7 +251,7 @@ SQL: - 差分を小さくするために、コードベースの改善活動を止めるのは本末転倒である（良いモノを作るためにコードレビューをしている面がある） - 必要に応じて、プルリクエストの分割を検討する -## 6. ファイルの移動やリネームと同時に大きく編集しない +## ファイルの移動やリネームと同時に大きく編集しない リファクタリングや構成の整理で、ファイルの移動やリネームを行う場面は多々ある。 @@ -264,7 +264,7 @@ SQL: - `git mv` で操作してからファイルを変更する - ファイルの移動やリネームで、`git commit` し、その後にファイルの変更を行う -## 7. 作業途中のプルリクエストはDraftにする +## 作業途中のプルリクエストはDraftにする 何かしらの理由で、マージされたくない作業途中の状態で、プルリクエストを作成したい場合がある。例えば、作業途中の内容を部分的にメンバーに確認してもらいたい場面や、セルフチェックが完了前に別タスクを行う必要が出た場合などがある。誤ってレビューやマージされたくない @@ -278,16 +278,16 @@ SQL: 作業途中であっても、早期にDraftプルリクエストを作成することで、作業の可視化／透明性に繋げることができる。他にも開発者が作業を再開しやすくなるなどメリットが大きいため、ローカルでの開発作業が完了する前の段階でDraftプルリクエストを利用することを推奨する。 ::: -## 8. プルリクエスト本文にチケット番号や関連情報を貼る +## プルリクエスト本文にチケット番号や関連情報を貼る トレーサビリティのため、チケット番号（のURL）や、設計検討したドキュメントのリンクや、議論スレッドのURLなどを、チケット本文に記載することで、後からトレース可能にする。チリツモであるがチームの開発生産性を上げることに繋がるため。 -## 9. チケット番号があるのであれば、プルリクエスト本文に重複記載は不要 +## チケット番号があるのであれば、プルリクエスト本文に重複記載は不要 作業目的（どういった要望／不具合に対する対応か）などは、作業チケットに記載されているのであれば、プルリクエスト本文での記述は不要である。 なお、サマリ事項だけ転記するなど、詳細な運用ルールはチームごとに決めて良い。 -## 10. ローカルでの動作確認ログなど、長くなる場合はMarkdownの折りたたみを利用する +## ローカルでの動作確認ログなど、長くなる場合はMarkdownの折りたたみを利用する GitHubでは `
` タグで[折りたたんだセクション](https://docs.github.com/ja/get-started/writing-on-github/working-with-advanced-formatting/organizing-information-with-collapsed-sections)が作成可能である。例えば、単体テストケース化できないような動作確認を行い、かつ動作検証ログをエビデンスとして貼り付けるような場面では、折りたたんだセクションを活用することで、本文を簡潔に保つことができる。 @@ -295,7 +295,7 @@ GitHubでは `
` タグで[折りたたんだセクション](https://do [スラッシュコマンド](https://docs.github.com/ja/issues/tracking-your-work-with-issues/using-issues/about-slash-commands)を用いることで、簡単に折りたたみできる詳細領域を挿入できる。入力時に `/` を押すとメニューが表示されるため、「Details」を選択する。 ::: -## 11. 画像をエビデンスとして貼るべきかどうか +## 画像をエビデンスとして貼るべきかどうか [2023年5月9日のアップデート](https://github.blog/changelog/2023-05-09-more-secure-private-attachments/)で、プライベートリポジトリのプルリクエストなどにアップロードされたファイルの閲覧に、認証が必須となった（それまではURLを知っていれば無認証でアクセスができてしまっていた）。 @@ -308,7 +308,7 @@ GitHubでは `
` タグで[折りたたんだセクション](https://do - [Future社員が使っているWindows便利ツール（新人さん向け） | フューチャー技術ブログ](https://future-architect.github.io/articles/20220107a/#ScreenToGif) -## 12. Files changedで余計なファイルがコミットされていないか確認する +## Files changedで余計なファイルがコミットされていないか確認する ジュニアメンバー／シニアメンバー問わず、特に新規参画した当初は新しい環境に慣れるのに手一杯で、誤った操作をしがちである。よくあるミスには、余分なファイルをコミットに含めてしまうことや、誤ったブランチからブランチを作成してしまったため、余計な差分が発生してしまうことなどがある。 @@ -318,7 +318,7 @@ GitHubでは `
` タグで[折りたたんだセクション](https://do このように、 `Files changed` を見ると、ローカルのエディタで見るのとは違ったセルフレビューの効果があるため、レビュアーになった気持ちで変更内容を確認することも推奨する。 -## 13. Files changedで特に見て欲しいポイントを補足する +## Files changedで特に見て欲しいポイントを補足する プルリクエストで、特に念入りに確認して欲しいポイントや、リファクタリングなど複数のコミットが混ざり、メインの改修ポイントがわかりにくくなってしまう場合がある。後者はコミットごとにレビューすれば解決するケースもあるが、featureブランチはスカッシュマージするチームでは、必ずしもきれいなコミットを求めないケースも多い。 @@ -335,13 +335,13 @@ GitHubでは `
` タグで[折りたたんだセクション](https://do ::: -## 14. Assigneesは自分を設定する +## Assigneesは自分を設定する プルリクエストのAssigneesは基本的にはレビュイー本人にする。もし、作成したプルリクエストを別の担当者に引き継ぐときは、Assigneesも変更する。これにより、最終的にどの開発者がどのような作業をしたか可視化しやすくなる。 Assigneesで指定された人が、そのプルリクエストの責任を持つ。つまり、マージさせるかクローズさせるか最終的に意思決定されるよう、推進する必要がある。 -## 15. Reviewers設定はレビュー依頼時にブランクで良い +## Reviewers設定はレビュー依頼時にブランクで良い Reviewersに設定することで、レビュアーが自身のアサインに気が付きやすくするメリットや、後からどれくらいレビューしたか把握しやすくできる。 @@ -351,7 +351,7 @@ Reviewersに設定することで、レビュアーが自身のアサインに - チームのだれがレビューするか不明な場合は、ブランクにする（候補者を片っ端からReviewersに設定しない） - CODEOWNERSで指定したファイルを編集したなどで、自明であればオーナーをReviewersに追加しておく -## 16. CIの成功を確認する +## CIの成功を確認する ローカルでLinterやテストが通っていると、プルリクエスト作成後、すぐにレビュー依頼を行いたくなる。しかし、何かしらの理由でレビュアーが確認した際にCIによるチェックが失敗している場面もよく見る。 @@ -363,7 +363,7 @@ Reviewersに設定することで、レビュアーが自身のアサインに - レビュアーとして、最初のフィードバックは「CIが落ちているので修正して欲しい」となり、余計なやり取りのホップが発生するだけであるため -## 17. レビュー依頼時にURLを貼る +## レビュー依頼時にURLを貼る プルリクエストのタイトルや、`#511` などの番号だけで依頼を出すのではなく、URLをSlackなどチャットに貼ることを推奨する。 @@ -373,7 +373,7 @@ Reviewersに設定することで、レビュアーが自身のアサインに - 後々、該当のプルリクエストのURLでSlackなどチャット検索が可能とするため - 前提条件など、Slackのスレッド上で確認しているやり取りの有無を確認したい場面がある -## 18. レビュー依頼はメンションを飛ばす +## レビュー依頼はメンションを飛ばす コードレビュー待ちは他作業が捗らないことが多いし、別作業に移ったとしても集中したタイミングでレビューフィードバックがあり、集中が途切れることもしばしばある。 @@ -388,7 +388,7 @@ Reviewersに設定することで、レビュアーが自身のアサインに レビュー対応が完了した場合も、再依頼が必要な場合はメンションを飛ばす方が良い。レビュアーとしても再度見て欲しいのかどうかは関心事であるためである。 ::: -## 19. Markdown記法を活用する +## Markdown記法を活用する Markdown記法を活用することで、Descriptionやレビューコメントを視覚的に整理し、意図を明確に伝えることを推奨する。 @@ -396,7 +396,7 @@ Markdown記法を活用することで、Descriptionやレビューコメント - [基本的な書き方とフォーマットの構文 \- GitHub Docs](https://docs.github.com/ja/get-started/writing-on-github/getting-started-with-writing-and-formatting-on-github/basic-writing-and-formatting-syntax) -## 20. マージはレビュイーが実施する +## マージはレビュイーが実施する [Gitブランチフロー規約](https://future-architect.github.io/coding-standards/documents/forGitBranch/git_branch_standards.html#%E3%83%9E%E3%83%BC%E3%82%B7%E3%82%99%E3%81%AF%E3%81%9F%E3%82%99%E3%82%8C%E3%81%8B%E3%82%99%E8%A1%8C%E3%81%86%E3%81%B8%E3%82%99%E3%81%8D%E3%81%8B)で記載された推奨ルールに従う。チームの自律性と生産性を重視するのであれば、レビュイーがマージすべきである。 @@ -413,7 +413,7 @@ typo修正など大したことがない修正であれば、やり取りを減 # レビュアーの推奨行動 -## 1. グラウンドルール +## グラウンドルール **レビューはコミュニケーションである。コメント記載時には相手への敬意を忘れないこと。** @@ -423,7 +423,7 @@ typo修正など大したことがない修正であれば、やり取りを減 - 参考: [プルリクエストを見る時、出す時に重要なマインドセット \- NRIネットコムBlog](https://tech.nri-net.com/entry/important_mindset_for_pull_requests) -## 2. レビュー依頼へのリアクションはできるだけ即レス +## レビュー依頼へのリアクションはできるだけ即レス 本規約の推奨は以下。 @@ -443,14 +443,14 @@ typo修正など大したことがない修正であれば、やり取りを減 ::: -## 3. Reviewersを自分に設定する +## Reviewersを自分に設定する プルリクエストに対して、レビューを実際に行う場合はReviewersに自分を選択する。以下の理由がある。 - 後からどのプルリクエストに対してレビューを実施したか、トレース／可視化しやすくする - 他のレビュアーが、◯◯さんが見るなら終わってからレビューを行う／スキップするという判断材料を提供する -## 4. hotfixの場合はマージ先のブランチを確認する +## hotfixの場合はマージ先のブランチを確認する 通常のプルリクエストであれば、featureブランチはデフォルトブランチ（多くはdevelopブランチ）がデフォルトで選択されるため、間違えは少ない。一方でhotfixブランチの場合は、main、developの2ブランチにマージ先がありえるため間違えやすい。 @@ -460,7 +460,7 @@ typo修正など大したことがない修正であれば、やり取りを減 複数のリリースバージョンを並行して開発する場合、一時的に develop、develop2 と複数のアップストリームを保つ場合がある。このケースは特にマージ先を間違えやすいため注意する。 ::: -## 5. 「Start a review」と「Add single comment」の使い分け +## 「Start a review」と「Add single comment」の使い分け GitHubでのコードレビューは、`Start a review` を行うと `Submit review` しない限り、レビューコメントが自分以外のメンバーが参照できない。 @@ -475,13 +475,13 @@ GitHubでのコードレビューは、`Start a review` を行うと `Submit rev - レビュイーとして、今レビューをしてもらっていると気付くことができ、心理的に楽になる - コメントを貰った部分の疑問点を即返信しレビュアーにフィードバックしたり、随時修正できる -## 6. レビュイーの思考を予想する +## レビュイーの思考を予想する レビュアーは、レビュイーが気がついていないかもしれない観点や影響範囲を意識して見るべきである。 レビュイーの経験やスキルセットから、不足していると考えられる面を重点的に確認することで、レビューの質を上げることができる。 -## 7. 暗黙知を形式知にする +## 暗黙知を形式知にする レビュー対象の変更内容が、機能上は問題なくても、他の実装方針と揺れている場合が存在する。 @@ -492,7 +492,7 @@ GitHubでのコードレビューは、`Start a review` を行うと `Submit rev 暗黙知を形式知にすることで、チームの方針が明文化され、透明性のある運営に繋げることができる。新規参画者にとってのオンボーディング負荷が下がり、戦力化が早まるメリットがある。 -## 8. レビューコメントにラベルを追加する +## レビューコメントにラベルを追加する レビューコメントにはラベル（プレフィックス）をつけることを推奨する。レビューコメントの意図を明確にすることで、開発者間の非同期的なコミュニケーションを円滑にし、作業効率を向上させる。 @@ -522,7 +522,7 @@ GitHubでのコードレビューは、`Start a review` を行うと `Submit rev 参考: [コード品質向上のテクニック：第51回 確信的な質問 | LINEヤフー](https://techblog.lycorp.co.jp/ja/20241121icq) にも、レビューアの「要求」であるのにもかかわらず、単なる「質問」にも読み取れてしまうミスコミュニケーションの例があり、 コードレビューにおいては要求と質問は明確に区別できるように書かれるべき とある。 -## 9. 適度なユーモアを取り入れる +## 適度なユーモアを取り入れる 心理的安全性を担保しつつレビューを楽しい場とするためには、ポジティブな絵文字（🚀✨️💯）を使うとともに、レビューコメントに適度にユーモアを入れると良い。 @@ -541,7 +541,7 @@ GitHubでのコードレビューは、`Start a review` を行うと `Submit rev ::: -## 10. 指摘内容は具体的であればあるだけ良い +## 指摘内容は具体的であればあるだけ良い レビューコメントで「可読性が低いです」 といったコメントは、レビュイーが受け取るとどのように修正して良いか判断ができず、次のアクションに繋がりにくいため不毛である。そのため、面倒であっても、どのように直して欲しいか、できる限り具体的に指示する方が建設的である。 @@ -550,19 +550,19 @@ GitHubでのコードレビューは、`Start a review` を行うと `Submit rev - 「可読性が低いです」 「{任意の非機能要件}を満たしていません」といったコメントで終始せず、具体的にどの部分を、どのように修正して欲しいかをフィードバックする - もし、修正する部分を（教育目的などで）レビュイーに考えてほしい場合は、その旨をコメントに追記する。レビュアーのお気持ちあてゲームにしないこと -## 11. 出典があれば追記できるとベター +## 出典があれば追記できるとベター 指摘内容の補足情報があれば追加するとベターである。例えば、「◯◯の観点で、このコードは△△の懸念があるため、□□□になるように修正をお願いします。公式ブログの{URL}にも記載があり、参考にできると思います。」といった形式である。 レビュイーが自分で調べれば事足りる場合もあるが、レビュアー側が出典を示すことで納得感が増す。また、誤った記事（過去のバージョンの記事や、見当違いの記事）を読んでしまい空回ってしまうことも、探す手間も減る。 -## 12. 指摘内容を論理的に説明できないのであれば、コメントすべきではない +## 指摘内容を論理的に説明できないのであれば、コメントすべきではない 「なんとなく汚いです」は指摘ではない。悪いコードだと感じたのであれば、相手が納得できるように論理的に説明する。論理的に説明できないのであれば、指摘をすべきではない。 - 引用: [デキるプログラマだけが知っているコードレビュー7つの秘訣 | PPT](https://www.slideshare.net/rootmoon/7-37892729) p26 -## 13. なるべく1度のレビューで指摘し切る +## なるべく1度のレビューで指摘し切る レビュイーからすると、レビューの「依頼1→指摘1→対応1→確認依頼1→別の指摘2→対応2→確認依頼2→別の指摘3→…」と、依頼のたびに指摘をもらうとタスクの予実管理も作業見積もりも建てられずストレスである（ムービング・ゴールポストである）。 @@ -571,7 +571,7 @@ GitHubでのコードレビューは、`Start a review` を行うと `Submit rev - できるかぎり最初のレビューで、指摘事項を出し切る - もし、2回目以降のレビューで追加要素に気がついた場合は、「\[imo\] 1度目の指摘で見逃してしまってすいません。この部分ですが、～の観点のため修正してもらいたく。」などと非を認めつつ、コメントする -## 14. 本筋から離れるコードコメントやREADMEのtypoなどはsuggestにする +## 本筋から離れるコードコメントやREADMEのtypoなどはsuggestにする プルリクエスト上のコメントで、レビュイーに対して直接変更内容を提案することができる[Suggested changes](https://docs.github.com/ja/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/incorporating-feedback-in-your-pull-request)という機能がある。 @@ -587,7 +587,7 @@ Suggested changesを使わないほうが良い場合もある。 参考: [君は GitHub の Suggested change を知っているか？ \- BASEプロダクトチームブログ](https://devblog.thebase.in/entry/2018/12/02/130657) -## 15. nits（WANT以下）のコメントだけであれば、先にapproveする +## nits（WANT以下）のコメントだけであれば、先にapproveする 対応が必須ではない、いわゆるnitsな指摘事項だけの場合がある。 @@ -599,7 +599,7 @@ Suggested changesを使わないほうが良い場合もある。 - 先にapproveすることにより、nitsであるがその対応可否をまったく検討せず、セルフマージするようなメンバーがいた場合は、対応状況を見極めてからapproveしても良い - つまり、初回、2回目は信頼してapproveすべきである -## 16. レビュアーはローカル環境で動作確認をすべきか +## レビュアーはローカル環境で動作確認をすべきか レビュー対象のリモートブランチを、ローカル環境にてテストなど動作確認すべきか迷う場合がある。レビュアーがローカルで動作検証を行うと、おそらく品質は高まるはずだが、レビュイーとの作業重複でもあるため工数は非常に高くなり、費用対効果は必ずしも良いと言えない。 @@ -610,7 +610,7 @@ Suggested changesを使わないほうが良い場合もある。 - CIで単体テストが成功／失敗しているかは確認できずはずである - どのように品質を担保するかはテスト戦略に依存させるべきである（その後のデプロイメント環境での結合テスト、システムテスト、性能テスト、障害テストなどに含まれるはずである） -## 17. 疑問点は臆せず質問する +## 疑問点は臆せず質問する レビュアーは意図が掴みきれない部分があった場合は、そのままにせず理解できるように務める必要がある。分かったふりをすると、後でその部分が火を噴くことが多いからである。また、「チーム全員が同じ知識を共有する」というコードレビューの意図からすると、レビュイーだけ分かっていればよいという状態は不健全である。レビュイーがリーダー（テックリード）であっても臆する必要はないし、レビュイーが自分より若手であったとしても、自分が調べても分からないことを聞くことは恥ではないので、堂々と教えてもらう。 @@ -620,7 +620,7 @@ Suggested changesを使わないほうが良い場合もある。 - 不明点が多い場合は、同期的なコミュニケーション（リモート会議。SlackのハドルやGoogle Meet）で確認する。やり取りのホップ数をなるべく減らすことを意識する - リモート会議を行う場合は、レビュアー側から時間や枠を調整する -## 18. 自信がない場合は、有識者にディスパッチする +## 自信がない場合は、有識者にディスパッチする レビュアーという立場だからといって、必ずしも開発チームが利用する全技術スタックや、開発プロダクトの過去の経緯に精通している訳ではない。それでも、自信が無い領域についてコードレビューを求められるケースも多々ある。 @@ -630,7 +630,7 @@ Suggested changesを使わないほうが良い場合もある。 - 分からないことは恥ではない。それより、知ったかぶりで手違いが発生する方がチームにとってのリスクである - 業務要件／機能要件が不明確であれば、プロダクトリーダー（業務担当者）に移譲する。非機能面で自信が無ければ、アーキテクトに移譲する。チーム規模によっては恒常的にこれら2つの役割のレビュアーで分担してレビュー方式も考えられる -## 19. ジュニアメンバーに対して大量の指摘がある場合にどうするか +## ジュニアメンバーに対して大量の指摘がある場合にどうするか 対象領域のスキル不足などが原因で、レビュー事項が大量にある場合に対応をどうすべきか迷う場合がある。 @@ -651,7 +651,7 @@ Suggested changesを使わないほうが良い場合もある。 - そのプルリクエスト内でレビュイーがキャッチアップしきれない可能性があるため、レビュアー側で補助した方がベター - 横展開が漏れることが多く、余計なやり取りが発生し、レビュアー・レビュイーともにストレスになることが多いため -## 20. レビュー終了の連絡 +## レビュー終了の連絡 レビュー終了した場合、レビューが終わった旨をプルリクエスト上でメンションをつけてコメントしたり、Slackなどのチャットで通知することで、レビュアーに対応を促すことができる。「Add single comment」の場合は、レビュー終了の切れ目が分からないためである。