はじめに
こんにちは、42tokyo Advent Calendar 2021の2日目を担当する、42tokyoの在校生、keiteanこと、kohkuboです。
42のカリキュラムには個人課題だけではなく、チーム開発が組み込まれています。今回、Shellを自作する課題で一緒に組ませて頂いたywakeさんからチーム開発について多くの学びがあったので、振り返りも兼ねてまとめます。
IOマルチプレキシングに対応したWeb Serverを3人チームで自作する課題で得た知見も追記しています。rakiyamasaさん、ynakamotoさん、チーム課題では協力していただき、ありがとうございます。
チーム開発の成果物はこちらです。
kohkubo/minishell
kohkubo/webserv
GitHubを利用したチーム開発
前提知識のフォーク、プルリクエスト、マージについてはこの記事では書きません。他のサイトでわかりやすく説明されているので、そちらに譲ります。
なぜチーム開発でGitHubを使用するのか
GitHubを使用すれば、コードベースのやり取りをすることができます。実装した範囲が一目でわかり、効率のよいレビューが行えます。またGitHub社が実践している開発フローGitHub Flowに従うことでチーム開発における困難の多くを解決することができます。
GitHub Flow
GitHub Flowはフォーク、プルリクエスト、マージのルールです。GitHubでの開発は以下の一連の処理によって行われます。
- ブランチの作成
- 実装
- プルリクエスト
- レビュー
- マージ
- ブランチの消去
今回は、そのルールをチーム開発向けにアレンジして取り組みました。
今回設けた6つのルール
今回の開発ではGitHub Flowを意識した6つのルールを設けました。
これらをベースにレビューや自動テストを行いました。設定のいくつかはGitHubのSettingで自動でチェックするようできます。
1. mainブランチは常に正しく動く状態にする
ブランチには直接コミットしないようにします。常にプルリクエスト経由でマージし、レビュー済み、テスト済みのコードが入るようにします。
2. 作業用ブランチは最新のmainブランチを必ず反映する
プルリクエストを出す前に、mainの変更をマージするようにします。
3. 作業用ブランチはissue単位で作成する
issueが細かい分には問題ありません。単位が大きいとレビューするのが大変になります。チームによりけりだと思いますが、目安を上げておくなら、ひとつのissueの実装に8時間以上かかりそうなら分割するのが良いと思います。issueは機能単位ではなく関数単位にしてしまっても良いです。実装がわからない場合、調査や設計もissueにするのもありです。
4. テストを必ず通す
GitHubのGitHub Actionsという機能を使って、プルリクエストを出すタイミングでそれまでの実装範囲すべてに必ずテストをするようにしました。これにより、新たに加わる実装が今までの実装に影響を与えていないかのチェックをすることができるようになります。
5. プルリクエストのレビューは他のメンバーが行う
mainへのマージは他のメンバーの承認を必須とします。今回は2人開発だったのでお互いにコードをレビューしてからmainにマージするようにしました。
6. プルリクエストが承認されたら、ブランチは削除する
レビューが通った場合は、mainブランチに変更をマージし、プルリクエストに使用したブランチを消去します。
ローカルでも以下のコマンドでブランチを消去します。
git branch -D <消去するブランチ名> # マージされていない変更があっても強制消去
mainブランチをルールで保護する設定
上記のルールをGitHubのレポジトリ設定で機械的にチェックするようにして、ブランチを保護します。ルールの設定はレポジトリのSettingsから行うことができます。
[ Settings ] -> [ Branches ] -> [ Branch protection rules ] -> Default branch 横の [ Edit ] を押します。
今回設定した内容です。
- コミットによるmainへの更新を拒否する (プルリクのみ受け入れる)
- レビューされていないコードの受け入れを拒否する
- 最新のmainを反映していないコードを拒否する
- 指定した自動テストに失敗したコードを拒否する
- 管理者であっても上記のルールを適応する
設定項目です。
自動テストの設定
GitHub Actionsでテストを自動実行するようにします。
GitHubには、指定したタイミングでシェルスクリプトを走らせるGitHub Actionsという機能があります。この機能を使って自動テストを行うことで、レビューをしてもらう前に、うっかりミスやバグの混入を機械的にチェックしましょう。
自動テストの際には、今回の実装範囲だけではなく、今まで書いたすべてのテストを走らせます。これで、今まで行われた実装に対して影響がないことが証明でき、レビューの際には新しく実装された範囲のレビューに集中することができます。
GitHub Actionsの書き方は別途記事にまとめています。
GitHub github actionsサンプルコード - Qiita
自動テストが失敗するとこのようにエラーが出て、マージがブロックされます。
テストの書き方
Google Testを使用したテスト駆動開発がおすすめです。ユニットテストが書きやすいです。別途記事にまとめました。同じく42のyaitoさんに教えてもらった環境設定含めて簡単に行えるようコードについて書いています。
C++向けの記事として書きましたが、少し変えるだけでCでも使えるので、参考にどうぞ。
Makefileに書くだけでOK！ C++で簡単にGoogle testを始めよう - Qiita
issueを有効活用しましょう。
Labelの有効利用
私のチームでは以下のLabelを定義していました。
- 00優先度:低
- 01優先度:中
- 02優先度:高
- 軽い
- モブプロ
Labelを元に担当を決めたり、週ごとのタスクを割り振っています。基本的に優先度の高いものから実装をしていく流れです。
Milestoneの使用
一週間ごとにMilestoneを設定していました。それぞれのアサインを週の頭に話し合います。
達成度合いが出るのでどれくらいの進捗かわかりやすいです。
実装に取り組んでみて、予想以上に工数がかかりそうだとか、予定までに終わらせられないとなったら、issueにコメントして別締め切りにするか、issueを更に小さなissueに分割するなどします。
基本的にweekの締め切りを変更する場合はメンバーに確認を取っていました。
issueをまとめるissueを作る
1つのテーマにイシューが複数立つ場合は、以下のようにチェックボックスを使ってイシューをリスト形式で作成 -> 右に出る「Open convert to issue」をクリックすることで、issueをまとめて作成することができます。
実装についての共通認識を作る
データの動きを軸にする
最後に、GitHubとは関係ありませんが、コードの説明を求められたときは、データの状態を軸にして話すことを意識しましょう。
設計はデータの受け渡し、整形、格納が軸になります。データがどのように整形されて、受け渡されるのか、データの形が決まっていれば、実装の依存関係が整理され優先順位付けもしやすくなります。
実装をモジュール単位で切り出して、それぞれがAPIとして機能するように考える。と説明すると伝わりやすい気がします。
1関数1役割を守る
命名規則にこだわることで、曖昧な実装をレビューの段階で指摘することができるようになります。レビューでは、その関数が命名にそった動きをしているか、チェックするようにしましょう。一つの関数が複数の役割を担っているときは、適切な役割を割り振るか、命名自体を適切なものに変更しましょう。
凝集度について理解を促す
可読性、見通しの良さ、というような抽象的な言葉は各人の理解がバラバラでコミュニケーションの妨げとなります。
リファクタリングの理由を説明するときに凝集度や、構造化プログラミングの用語を共通認識として持つようにし、感覚的な言語の使用を避けるようにします。
YAGNI原則を守る
後で使うかも、と実装しがちですが、現状いるものだけ実装しましょう。
YAGNI原則（You Ain't Gonna Need It）とは - 意味をわかりやすく - IT用語辞典 e-Words
その他の小ワザ
GitHub Webhooks
GitHub Webhooksを設定し、Discordで通知を受け取りましょう。
GitHubの更新通知をDiscordに送ることで、新たなissueやプルリクエストの対応が素早くできるようしました。Discordでなくても、他のSNSでもいろいろ連携できます。
GitHub Projects
今回は別ツールを使ったのですが、優先順位を相談したり、お互いの課題の進捗を確認するのには、GitHub Projectsが使えます。
フォーマットや命名規則についてツールを使用する
C/C++では、以下のツールでフォーマットを強制するとレビューの際に細かい指摘をしなくてよいので楽です。
- clang-format
- clang-tidy
明文化して逐一指摘するのではなく、ツールを使ってGitHub Actionで強制しましょう。
言葉で補足する
基本は以下です。
- コードには How
- テストコードには What
- コミットログには Why
- コードコメントには Why not
コミットメッセージに関してはツールを使っていました。
commitizenです。このツールを使えば、質問に答えて行くだけで、OSSにも使われているようなしっかりしたコミットメッセージを書けるようになります。
書き方で迷ったらAngularの規約のようなOSSのコミットメッセージのガイドラインが参考になります。
プラスαのルールを作る
42で一緒に学んでいるrnakaiさんと、kkamashiさんが明文化されていたプラスαのルールがよかったので一部を引用します。特にレビューコメントの仕方をルールに入れているところがいいですね！
- １ファイルに無理に関数詰め込まない。見通しやメンテナンスの観点でも、ファイルは適切に分割する。
- ファイル名と関数名は極力一致させる（ファイル名の情報から、簡単に関数を見通せるようにする）
- ディレクトリは機能がわかりやすいようにファイルをまとめる。
- ヘッダファイルはincludesディレクトリ以下に入れる
- ヘッダファイルは関数の目的・機能別で分割する。ディレクトリ単位などでまとめるのが好ましい。ディレクトリを横断するような機能を持つ関数はその都度判断し、レビューにより合意形成する。
- 特に機能追加などのコードレビューではノーコメントは避ける。（小さなバグ修正など、マイナーチェンジだけのPRなどはこの限りではない）
目標の明確化
プロジェクトを始めるとき、最初のissueでQuality(品質), Cost(予算), Delivery(納期), Scope(対象範囲)を明文化してしまいましょう。
それと合わせて、チームメンバーの目標も明確になっていると、作業分担を考えるときに参考になります。
まとめ
この記事は、今回一緒にチームを組んでいただいた、ywakeさんから学んだことがベースになっています。ywakeさん、改めて、ありがとうございました。
Web Serverを作る課題でもrakiyamaさん、ynakamotoさんに協力してもらいました。ありがとうございました。
この記事が良かったと思う人はLGTMボタン、ユーザーのフォローもよろしくね！
明日は、mhirabayさん(@bayamasa)が「CSAPP読んだ感想とまとめ」について書いてくれる予定ですので、そちらの記事もお楽しみに！