大切なことはすべてGitHubが教えてくれた〜女将流コードレビュー〜
この記事は CivicTechテック好き Advent Calendar 2020 の14日目の記事です。
こんにちは。GitHub女将こと今村(github @kaizumaki)です。東京都新型コロナウイルス感染症対策サイトの開発に関わってから、早10ヶ月がたとうとしています。
最初は純粋にフロントエンドエンジニアとしての興味からでした。ただ、開発の内容は伏せられていましたので、本当に「JavaScriptが書ける」ということだけで飛びつきました(細かくは「TypeScriptが書ける」ことが要件だったのですが、当時TSは初心者ながら取り組み始めていたところでした)。
ちなみに私が初めてOSSにコントリビュートしたのはDjango Girls Tutorialのリポジトリでした。内容としてはチュートリアルなので、ソースコードではなくドキュメントなのですが、私としてはissueとPull Requestを介して英語で海外のコントリビューターたちとやりとりをするということに興奮しました。
そんな私がまさか、全国的に(いや世界的に!?)注目されるリポジトリのメンテナーとなるとは。
私は本業はwebエンジニアだと名乗っていますが、いわば何でも屋です。主にフロントエンド分野をやっていますが、phpを書くこともあるし、趣味ではPythonをやっていたりします。どれも深い知識はなく、広く浅くといったところです。
その「広く浅く」あった知識が、githubリポジトリのメンテナーとしての職能に合致したのではないかと、自分で分析しています。
3月下旬、Code for Sapporoの古川泰人氏から「GitHub女将」との呼び名をいただきました。
ここからは私がgithubメンテナーとしてどのようにコードレビューをやってきたかを綴ろうと思います。
issueとPull Requestのテンプレート
私は最初からgithubに詳しかったわけではありません。このリポジトリに怒涛のissueとPRが投げられていた3月に、githubの使い方を徐々に身につけていったというところです。
今になっても助けられているのがissueとPRのテンプレートです。OSSに詳しい初期メンバーとコントリビューターが作ってくれました。
これがあることで、提案者が何を求めているのかがとてもわかりやすくなりました。また、コントリビューターの皆さんもこのテンプレートに準じて丁寧に書いてくれるのでとても助かっています。
PRのテンプレートでは、
close #0 <!-- 関連するissue番号を書く -->
とすることで、Linked issuesを設定することができ、PRをマージした時には自動でそのissueが閉じられます。
レビューする時には、関連したissueとPRのコメントをよく読むようにしています。
NetlifyのDeploy previews
実は当初、私はこの機能に気づいていませんでした。ソースコードしか見ていなかったです。
このNetlifyの機能がなかったら、数々のPRをさばくことはできなかったでしょう。
女将流コメントの心得
提出されるPRはフロント実装に関わるものから、開発環境の整備に至るものまで、さまざまなソースコードがあります。正直、私はそれらすべてについてはわかりません。Deploy previewを見て、大丈夫そうだったらApproveしてマージ、としたいところです。
ただ私は「何かしらコメントを残す」ということを意識的にやっていました。ここはどうしても引っかかる、というコードを見つけてPR主に質問するのです。
例えば以下は「aタグのrelにnoreferrerを追加する」というPRに対しての私のコメントです。
自分でも当たり屋みたいなコメントだな、と思うのですが、要は後々になって「このコードはなぜこうなっているんだ?」と思った時のために記録に残しておくことが大事にしていたのです。
「わからないので教えてください」「もう少し詳しく知りたいです」とコメントすることで、PR主の意図を引き出すとともに、広く参照できるようにするのが目的です。
それと、「自分一人で判断しない」ということも大事にしていました。運営コアメンバー含め、コントリビューターの皆さんの知識と知恵をできる限り拝借したいという思いでした。そのために1つ1つのPRの判断に時間をかけました。
こんな時どうする?
デザインに関係する提案の場合
相談フロー図やグラフの表現に対する提案はたくさんありました。特にグラフの表現に対する提案で見送ってきたものの中には、個人的に採用したい案もありました。
ただ、東京都の公式サイトである以上、東京都の広報として何が相応しいかを考えなくてはなりません。そこには「データに解釈を加えない」という当初からのポリシーがありました。その点は守ってきてよかったと思っています。
そして一貫して、少しでも見た目の変更を伴う場合はすべて東京都の担当者の確認をとっています。
また、例えばある人がAという表現が望ましいと考えてPRを出したとして、それをマージした後に、別の人がBという表現にしたいとしてPRを出した時に、それをまたマージすると、見た目の上書き合戦になってしまいますよね。それこそ一貫性のないサイトになりかねません。そこは注意してブレーキをかけていたところはあります。
同じようなPRが複数ほぼ同時に提出された場合
見た目に関する提案で、似たような要望のPRが同時に提出されることが稀にありました。そんな時は本当に対応に悩みます。
もちろん、少しでも早く提出された方を採用すべき場合もあろうかと思います。ただ、その観点では先ほど書いたように見た目の上書き合戦になりかねません。
そんな時、私が一番に考慮するのは「コードがシンプルなこと」です。それはコードが短ければいいというわけでもありません。もちろん短いのはいいことですが、同時に「可読性が担保されること」も重要です。
その条件が揃うことで、OSSというエコシステムの中で広く参照されることに耐えうるソースコードになると思っています。
使わなくなったカンバン的タスク管理
プロジェクトの立ち上げ当初はTrelloといったタスク管理ツールやリポジトリでKanbanを作ったりしていましたが、ほどなくして全く使わなくなりました。
大量のissueとPRをさばくのに一番役に立ったのはLabelsです。これもOSSに詳しいコアメンバーが整理してくれたものですが、こういったコントリビューター間の共通認識があるのはとてもいいですね。
とある取材を受けた時にも、このラベリングの作業に注目してもらいまして、その後記事となりました。
GitHubを介したコミュニケーションから私は実に多くのことを学びました。
最後に、Software Design 2020年9月号に私が寄稿した原稿の中から、以下の文章を自ら引用して締めたいと思います。
「公式サイトの運営は辛いですね」と言われたこともあります。確かに、市民と行政の板挟みと取られるかもしれません。私としては板挟みに悩むというよりは、東京都とともにコントリビューターも守る、そういう割とシンプルな心持ちでいます。