Slide 1

Slide 1 text

1 Code Review in a Team Yuki Iwazaki@chck | AI Lab Press Space for next page AI Lab Skill Up Onboarding 2024

Slide 2

Slide 2 text

1 Prerequisites x git command が使える ( git -v ) x CyberAgentAILab/skillup2024-code-review にアクセスできる x CyberAgentAILab/skillup2024-code-review/projects で team-* Projects が見える 準備ができていない方も発表を聞いていれば一通り理解できるように作ってあります

Slide 3

Slide 3 text

1 Yuki Iwazaki @ chck 2014 … Backend Engineer in DSP 2017 … ML / DS in Ad Platform 2018 … Research Engineer in AI Lab github.com/chck Research Engineer | Applied ML

Slide 4

Slide 4 text

1 コードレビューとは What is a code review?

Slide 5

Slide 5 text

1 コードレビューとは A code review is a peer review of code that helps developers ensure or improve the code quality before they merge and ship it. 👍: 知識の共有 コードレビューに関わるメンバーの技術力を高めることができる。 👍: バグの早期発見 リリース後にリスクになるようなバグを未然に防げる。 👍: コンプライアンスの維持 コーディング規約への準拠を確認することで、複数人での開発に堅牢な実装を維持できる。 👍: セキュリティの強化 リリース後にリスクになるようなセキュリティ上の問題を防げる。 👍: コラボレーションの強化 情報の縦割り化を防ぎ、チームでプロジェクトを進めるためのコミュニケーションになる。 👍: コード品質の向上 Reviewer の知見を反映することでReviewee が見逃してしまう技術的負債を防げる。 🙅: コード提供の遅延 レビュー対応のやりとりによってコードの提供が遅れる。 🙅: レビューに時間がかかる コードレビューを優先する間、自身のタスクが進まなくなる。特に大規模コードのレビューは時間がかかる。 [1]

Slide 6

Slide 6 text

1 なぜ研究機関でコードレビューが必要か 実装力の向上 結果的に普段の研究開発サイクルを早く回すことができる 実験結果の再現性 再現性の高い研究コードは信頼性が高い セキュリティ・コンプライアンス 自分一人では気付かないリスクをチェックできる プロダクト・チーム連携 未来の自分も含め、共有しやすい実装ができる

Slide 7

Slide 7 text

1 チーム開発のお作法 共著、プロダクト連携、未来の自分 論文の締切まで実験計画の立案、タスク分担、進捗管理 Kaggle での実例 Kanban によるタスク管理、データ格納場所の一元化、評価方法の統一 タスク分担も各自が興味のあること(サーベイ、評価方法の模索、実験パイプライン実装)に集中 ≒ 複数人で仕事をするときのお作法 HTTPS://NOTE.COM/JDSC/N/N40EB522A9CFE HTTPS://NOTE.COM/YOUHEI0917/N/ND25DD2FF63E6

Slide 8

Slide 8 text

1 Hands-on

Slide 9

Slide 9 text

1 役割を決めよう 席を近づけて n 人組を作ってください 各グループ内でReviewee とReviewer に分かれよう Screen を見て一番左の方がReviewee それ以外は全員Reviewer Reviewee 人数 < Reviewer 人数になるように 作れたらReviewee 担当だけ挙手 🙌

Slide 10

Slide 10 text

1 研究計画を立てよう 締切までにやらなければいけないことを書き出す サーベイ データセット整備 手法の実装 実験結果のまとめ 執筆 今回は計画は既に立ててあることとします 2025-02-02 2025-02-09 2025-02-16 2025-02-23 2025-03-02 2025-03-09 2025-03-16 2025-03-23 サーベイ データセット整備 手法の実装 実験計画決め Section Another 研究計画2025

Slide 11

Slide 11 text

1 Kanban を作ろう CyberAgentAILab/skillup2024-code-review にアクセス、Projects をクリック Projects -> New project -> Featured: Kanban -> Project name: "$ チーム名" -> Create project これも作ってあることとします GitHub のProject 機能

Slide 12

Slide 12 text

1 チケットを切ろう チケットは3 つの状態 (Draft, Issue, Pull Request) から選べる Issue は「既存研究のサーベイ」など、必ずしも実装と紐づけなくてOK 🙌 Hands-on: 今回は以下3 つのチケットをDraft で切ります 既存研究のサーベイ データセットの準備 提案手法の実装 Kanban board によるTodo のチケット化 各用語の関係性 Kanban board └── Ticket (Item) ├── Draft ├── Issue └── Pull Request

Slide 13

Slide 13 text

1 チケットを切ろう Kanban board によるTodo のチケット化

Slide 14

Slide 14 text

1 チケットをIssue 化しよう チケットを優先度に分ける Backlog からReady に移動してみる チケットをIssue 化する

Slide 15

Slide 15 text

1 チケットを分担しよう Assignee を決める 今回は周りのメンバーでも自分でもOK チケットをクリックするとIssue に飛ぶので右メニューのAssignees から指名 データセットの準備、提案手法の実装で2 回やってみる

Slide 16

Slide 16 text

1 Branch を切ろう チーム開発や個人でも誰かへ共有予定のあるコードはレビューなくmain branch にpush してはいけない とにかくコードを変更したくなったらfeature branch を切ってそこで作業する main feature/issue-1 0-5478550 1-93316c9 2-06ccbf0 4-be68977 著名なBranch 戦略であるgit-flow をベースに、研究機関としてシンプルにしたもの main : 開発時のバグや実行時エラーが解消されている安定的なBranch feature/* : 新機能の追加や既存機能の変更を行うBranch

Slide 17

Slide 17 text

1 Hands-on: Branch を切ろう リポジトリをClone ※ $team にはチーム名を入れる 例えばteam blue の場合は git clone https://github.com/CyberAgentAILab/skillup2024-code-review.git cd skillup2024-code-review git checkout -b feature/issue-$team git checkout -b feature/issue-blue

Slide 18

Slide 18 text

1 Note: Product use develop や hotfix branch が増え、Production の事故を極力減らした構成に main develop feature/issue-1 hotfix/issue-2 0-534544e 1-2f01b85 2-7b282a6 3-345b864 6-fac3783 7-fdfbfa5 develop : main にマージするまで開発の中心となるBranch hotfix/* : main に対して緊急の修正を行うBranch 1. https://future-architect.github.io/coding-standards/documents/forGitBranch/git_branch_standards.html ↩︎ [1]

Slide 19

Slide 19 text

1 実装しよう 何よりも再現性と構造化 ラスタ画像、UI ポチポチなど非可逆的なフォーマットを極力減らし、差分を確認しやすい状態を作る Markdown, Mermaid, Terraform 手順の多いコマンド処理は自動化する Shell Script, Makefile, Makefile.toml, Taskfile.yml Project の構成を意識する ディレクトリ構造、モジュール分割、クラス設計 今回は既に実装済のコードが用意してあります Hands-on: 実装済のコードを引っ張ってくる Hands-on: 差分を確認する git pull --rebase origin feature/sample git diff main origin/feature/sample -- ':!docs'

Slide 20

Slide 20 text

1 Commit をしよう 今回は既にcommit 済のものを眺めてみる Hands-on: Commit 履歴を表示 Hands-on: Commit 履歴を簡略化して表示 このようにオプションが複雑なものはgit alias を設定しておくと便利 git log git log --graph --pretty=oneline --decorate --date=short --abbrev-commit --branches

Slide 21

Slide 21 text

1 Tips: Commit message に迷ったら Semantic Commit Messages: Google/Angular の規約にアレンジを加えたもの。実装内容によって7 types を使い分ける type description example chore 微修正 chore: fix typo docs ドキュメント関連の変更 docs: add readme how to run feat コードの本体機能に関する追加・変更 feat: modify dependencies for training fix バグ修正 fix: ensure loss function returns non-zero refactor パフォーマンス改善等のリファクタリング refactor: improve model loading style フォーマット等の見た目上の変更 style: apply ruff linting Commit Message の規約に従う feat: add hat wobble ^--^ ^------------^ | +-> Summary in present tense. +-------> Type: chore, docs, feat, fix, refactor, style, or test.

Slide 22

Slide 22 text

1 Pull Request (PR) を上げよう Pull Request: 実装内容をmain に取り込む (merge する) ためのリクエスト 作業途中でもいいのでDraft でPR を上げる こまめに進捗共有するのと同義 Draft もなくいきなり200 files のPR が来たらレビューコストも高い コミットを整理する PR の説明欄にはどんな変更をしたかを書く Issue とPR を紐付ける ( まとめられるならCommit でもOK) PR のテンプレートを活用する 見てほしい人をReviewer に指名する Reviewee 側のお作法

Slide 23

Slide 23 text

1 Hands-on: Pull Request (PR) を上げよう ※ $team にはチーム名を入れる 例えばteam blue の場合は CyberAgentAILab/skillup2024-code-review にアクセス、 main <- 該当Branch でCreate Pull request git push origin feature/issue-$team git push origin feature/issue-blue

Slide 24

Slide 24 text

1 Hands-on: Pull Request (PR) を上げよう 1. PR のDraft 化 2. 実装 ( 今回はskip) 3. レビューができる状態になったらReady for review ボタンでDraft を解除しReviewer にリクエスト

Slide 25

Slide 25 text

1 レビューをしよう 設計: コードはうまく設計され、そのシステムにとって適切か? 機能性: コードは作成者の意図通りに動作するか?ユーザーにとってコードの挙動は適切か? 複雑さ: コードはもっとシンプルにできるか? コードを作成した開発者があとになってもう一度そのコードに触れたとき、理解しやすく使いやすいか? テスト: そのコードには、正確で、うまく設計され、自動化されたテストがあるか? 命名: 変数名、クラス名、メソッド名などに明解な名前を選んだか? コメント: コメントは明解で有益か? スタイル: コードはスタイルガイドに準拠しているか? ドキュメント: 関連するドキュメントも更新してあるか? 1. https://fujiharuka.github.io/google-eng-practices-ja/ja/review/ ↩︎ 具体的な8 種類のレビューポイント[1]

Slide 26

Slide 26 text

1 レビューをしよう 礼儀正しく、コメントは丁寧に 🙅「並行処理にしてもメリットがないのにどうしてこんな実装をしましたか?」 👍「ここでの並行処理はパフォーマンスはそのままで処理を複雑にしているだけに見えるので、シング ルスレッドにする方がいいかもです」 そのコメントをする根拠も合わせて伝える なんとなく直してほしいではなく、参考文献を載せたり、その提案によってどう良くなるのかを解説して 説得力を補強する 指示なのか提案なのか質問なのかのニュアンスをはっきりする コメントはするもののすべて対応しなくてもいいことを前提とし、そのコメントはどれくらい優先度高 く対応してほしいのかをはっきり伝える 理解できないコードがある場合は、Reviewee に説明を追加してもらう Pull Request 上のやりとりだけでなく、コード上にコメントを残してもらうことも検討する レビューコメントの書き方

Slide 27

Slide 27 text

1 Tips: レビューコメントのラベル化 label description [must] 必須。明らかに実装が間違っているとき [imo] 私の意見では。直してほしいけど必須ではない [nits] 細かい指摘。typo とか [fyi] 参考までに。関連記事のURL を貼ったり [ask] 質問。実装の意図を深堀って聞きたいとき 例 [must] この依存は使われていないので消したほうが良いかもです [ask] このA 関数はB 関数とどうやって使い分ける想定でしょうか 優先度のニュアンスを伝える

Slide 28

Slide 28 text

1 Hands-on: レビューをしよう レビュー依頼が来たら自分のタスクに割り込んででも早く対応する 最長でも1 日以内、ボールを止めている意識を持つ 特に大きなプロジェクトの場合main branch がどんどん進んでしまいMerge が困難に Hands-on: PR 画面 -> Files changed -> Start a review -> Comment -> Submit review ファイル単位でレビュー済かどうかをチェックする機能 行を指定してコメントを残す機能 直接コードに修正案も書ける 1. https://fujiharuka.github.io/google-eng-practices-ja/ja/review/reviewer/speed.html ↩︎ [1]

Slide 29

Slide 29 text

1 Hands-on: レビューをしよう

Slide 30

Slide 30 text

1 Tips: Submit review で選べる3 択について レビューした内容のニュアンスをざっくり伝えることができる Comment: あくまでもコメント Approve: 内容良さそうなのでマージを許可 Request changes: マージ前に対応してほしい項目がある

Slide 31

Slide 31 text

1 Tips: 返信テンプレート機能 shields.io というbadge maker と組み合わせればkawaii 返信テンプレートを登録しておけて便利 Profile settings -> Saved replies

Slide 32

Slide 32 text

1 レビューコメントを反映しよう コメントを受け入れる場合 コミットメッセージのお作法を思い出す コメントを受け入れない場合 レビューコメントに対する自分の考えをコメントする(Rebuttal のようなもの) 対応後にもう一度Reviewer にリクエストを送り、LGTM をもらうまで繰り返す Reviewee 側のお作法

Slide 33

Slide 33 text

1 PR をマージしよう 基本的には PR ページ下の Merge pull request ボタンを押すだけ (コンフリクトが予想されるためこのハンズオンでは押さない) おつかれさまでした!

Slide 34

Slide 34 text

1 リリース(main merge )後の開発サイクルについて Document を書こう README.md やHelp Page の充実 Issue に返信しよう ユーザーは最大のdebugger 依存パッケージを定期的にアップデートしよう 古いコードでも今の依存で動くようメンテされていれば長く使ってもらえる リリースノートを書こう 最近のアップデートでどんな変更を行ったのか

Slide 35

Slide 35 text

1 まとめ Kanban を使ってタスク管理からPull Request のマージまでを体験 研究コードは普段からGit 管理をしよう 開発力/ コードレビュー力を上げるためにできること チーム開発をしよう 共著なら実験途中からGitHub を使おう ペアプロ、モブプロをしよう 単著でもお互いのProject code を見せ合おう お手本から学ぶ コードスニペットを集めよう LLM に訊くのも良いが、GitHub 内検索も有効 Reading 9 : Writing 1 詳しい人に訊こう

Slide 36

Slide 36 text

1 Thank you for listening! Questions?

Slide 37

Slide 37 text

1 Appendix

Slide 38

Slide 38 text

1 Note: 良いコードとは 動くコード vs 綺麗なコード 該当する品質特性 解決したい問題 動くコード 機能性 収益の向上 綺麗なコード 変更容易性 開発コストの低減 動くコード:綺麗なコードを書くのに時間を割いていられない 綺麗なコード:綺麗なコードを書かないと保守や変更が大変になる AI Lab のような研究機関における2 つの収益 a. 実験サイクルを高速化し、論文の締切に間に合わせ、トップカンファレンスに採択される b. 利用者に再現しやすいコードを公開し、研究成果をコミュニティに広める 1. https://developersblog.dmm.com/entry/2024/11/01/110000 ↩︎ [1]

Slide 39

Slide 39 text

1 Note: 幻のRefactor フェーズ 2024-10-20 2024-10-27 2024-11-032024-11-10 2024-11-17 2024-11-24 2024-12-012024-12-08 2024-12-15 2024-12-22 2024-12-292025-01-05 2025-01-12 2025-01-19 2025-01-262025-02-02 2025-02-09 2025-02-16 動くコード( R&D フェーズ) そこそこ綺麗なコード( R&D フェーズ) Paper Deadline/Production Deploy 綺麗なコード( Refactor フェーズ) 綺麗なコード( Refactor フェーズ) パターン1 パターン2 締め切りを起点にした研究コードの品質 パターン1 :動くコードから綺麗なコードへのRefactor コストが高い パターン2 :普段からそこそこメンテできているコードはRefactor コストが低い 締め切りを起点にした研究コードの品質

Slide 40

Slide 40 text

1 CI を活用しよう Ruff + pre-commit でコミット時にコード品質を保ちたい GitHub Copilot コード レビューの使用 GitHub Actions でPython のLint とTest (テスト)をとりあえず実行 GitHub Actions で Python コードの自動フォーマットを実現しよう

Slide 41

Slide 41 text

1 Tips: GitHub コマテク集 https://techblog.openwork.co.jp/entry/github-tips