こんにちは、kanaです。社内ではpixivというサービスでPHPとTypeScriptとVim scriptを書く仕事をしています。今日はpixivの開発におけるコードレビューの話をします。
問題
pixivは昨年でサービス開始から10周年を迎えました。サービス開始当初と比較すると山のように新しい機能や画面が増えています。なのでpixivのコードベースは巨大です。PHPファイルだけでも5000個以上あります。
kana@pixiv ~/pixiv (2) [master]
^-^)/> git ls-files '*.php' | wc -l
5555
これだけの数のファイルを全て把握するのは無理です。なので、各々の開発者が自由にコードを書いて混沌にならないよう、設計方針を始めとして各種コーディング規約が整備されており、秩序を保っています。
とはいえ誰もが常に完璧なコードを書けるわけではありません。なので何か変更をmasterにマージする為には、少なくとも2人のコードレビューを受けてLGTMを貰うルールになっています。これは、
- レビューでの指摘漏れを減らして問題を事前に食い止め易くする
- 自分がレビューで気付けなかったところを把握して、次回からより良いレビューができるようにする
- 各自に少しづつpixiv全体のコードを把握してもらって、いざ障害が起きた時に誰かが直ぐに対応できる状態にする
という理由があります。
ところで、pixivの開発に長く携わっているとpixivの様々な部分の実装やインフラ構成や障害事例への理解が進んでいきます。そうすると、
- 「この分野はkanaさんが詳しいからkanaさんにレビューをお願いしよう」
- 「この機能はkanaさんが作ったからkanaさんに確認してもらおう」
- 「このデータ構成どうなってるのか謎だ……昔から居るkanaさんに相談しよう」
と周囲の開発者から頼られるようになります。毎年新卒エンジニアがどんどん増えているので、それに比例して頼られる回数も増えます。
頼られるのは嬉しい事です。ただ、あまりに多くのレビュー依頼が飛んでくるようになると、相対的に自分の作業時間が減って困ります。
自分が忙しい時は他の適切なエンジニアにレビューを代わってもらいます。長期的には頼れるエンジニアを増やして自分へのレビュー依頼を減らしたいので、各自の興味分野を勘案してレビュー依頼を再割り当てするようにもしています。
とはいえ、短期的にも何か効果を上げられる施策は無いものでしょうか。
設計
あれこれ頭を捻った結果、
- 本当に自分へのレビュー依頼が多数飛んできてるのか可視化する
- 可視化により「自分は忙しいから他の人にレビューを依頼してね」オーラを出してみる
という事ができれば良さそうです。
ピクシブでは社内のコミュニケーションツールにSlackを利用しています。なのでSlackの自分のステータスに自分へのレビュー依頼数を出せば忙しさオーラを出せます。 具体的には以下のようなイメージです:
Slack標準のemojiだと 0 (:zero:) 1 (:one:) …… 9 (:nine:) 10 (:keycap_ten:) まで数えられます。
11以上になった場合は「とにかく沢山レビューしてるからもう勘弁して」という気持ちを込めて ∞ (:infinity:)にしておくと良さそうです(∞ は標準のemojiには無かったので追加しました)。
ただ、これだけだとカウントアップし続けて直ぐに ∞ になってしまうので、日付を跨いだらカウントをリセットする必要があります。 これを判定する為には「最後にステータスを変更した日付」を何処かに記録しておく必要があります。 ちゃんと記録するのは煩雑なので、ここはステータスの中に日付を表す文字列を埋め込んで済ます事にしましょう。
実装
カウンターを実現するには以下の要素が必要です:
- レビュー依頼が飛んでくるチャンネルの発言をチェックする
- 発言の内容に応じてSlackの自分のステータスを変更する
いずれもSlackのAPIを利用すれば実現できます。
後はこの2点を仲立ちするサーバーを用意すれば完成ですね。
レビュー依頼が飛んでくるチャンネルの発言をチェックする
これはSlackのEvent APIを利用すればできます。 Slack側での設定のポイントは以下の3点になります:
- Slack AppにBot userを紐付けておく
- チェックするイベントはBot User Eventの
message.channelsにする - ここで用意したBot userをチェックしたい各チャンネルにinviteしておく
チャンネルの発言をチェックするだけならBot userを追加する必要はありません。
Workspace Eventの message.channels で事足ります。
ただ、このイベントは「自分が入っている全ての公開チャンネルのメッセージ」について送られるので、 あっという間にrate limitに引っかかってしまいます。
実際にレビュー依頼が飛んでくるのは極一部のチャンネルに限定されます。 Slack AppにBot userを紐付けておけば、そのBot userが入ったチャンネルのイベントのみ送られるようになります。
発言の内容に応じてSlackの自分のステータスを変更する
これはSlackのWeb APIのうち
users.profile.get
と
users.profile.set
を使えばできます。
これらのAPIを利用するには以下の権限をSlack Appに付与しておきます:
users.profile:readusers.profile:write
上記2点を行うサーバーを用意する
例えばNodeで書くと以下のような感じになります:
| const bodyParser = require('body-parser') | |
| const express = require('express') | |
| const moment = require('moment-timezone') | |
| const slack = require('slack') | |
| const VERIFICATION_TOKEN = process.env.VERIFICATION_TOKEN | |
| const OAUTH_ACCESS_TOKEN = process.env.OAUTH_ACCESS_TOKEN | |
| const app = express() | |
| app.use(bodyParser.json()) | |
| app.set('port', process.env.PORT || 5000) | |
| app.get('/', (req, res) => { | |
| res.send('Hello') | |
| }) | |
| app.post('/', async (req, res) => { | |
| if (!req.body) { | |
| res.end() | |
| console.log('missing body') | |
| return | |
| } | |
| if (req.body.token !== VERIFICATION_TOKEN) { | |
| res.end() | |
| console.log('invalid token') | |
| return | |
| } | |
| if (req.body.challenge) { | |
| return res.send(req.body.challenge) | |
| } | |
| if (req.body.event && req.body.event.type === 'message') { | |
| res.end() | |
| await handleMessage(req.body.event.text || '') | |
| return | |
| } | |
| res.end() | |
| }) | |
| app.listen(app.get('port'), () => { | |
| console.log('Node app is running on port', app.get('port')) | |
| }) | |
| async function handleMessage (text) { | |
| if (!text.match(/\b(kana|@kana|U1UP30NRL)\b/)) { | |
| return | |
| } | |
| const p = await usersProfileGet() | |
| const statusText = p.profile.status_text || '' | |
| const m = statusText.match(/^(\d+)個レビューしてる\((.*)\)$/) || [] | |
| const statusCount = parseInt(m[1] || '0', 10) | |
| const currentDate = moment().tz('Japan').format('M/D') | |
| const statusDate = m[2] || currentDate | |
| const moreCount = (text.match(/https:\/\/github\.com\/YOUR_ORGANIZATION\/[^\/]+\/pull\/\d+/g) || []).length | |
| const newCount = currentDate === statusDate ? statusCount + moreCount : moreCount | |
| const emojiTable = { | |
| 0: ':zero:', | |
| 1: ':one:', | |
| 2: ':two:', | |
| 3: ':three:', | |
| 4: ':four:', | |
| 5: ':five:', | |
| 6: ':six:', | |
| 7: ':seven:', | |
| 8: ':eight:', | |
| 9: ':nine:', | |
| 10: ':keycap_ten:' | |
| } | |
| const emoji = emojiTable[newCount] || ':infinity:' | |
| usersProfileSet({ | |
| status_emoji: emoji, | |
| status_text: `${newCount}個レビューしてる(${currentDate})` | |
| }) | |
| } | |
| async function usersProfileGet () { | |
| return await slack.users.profile.get({ | |
| token: OAUTH_ACCESS_TOKEN | |
| }); | |
| } | |
| async function usersProfileSet (statusData) { | |
| return await slack.users.profile.set({ | |
| token: OAUTH_ACCESS_TOKEN, | |
| profile: JSON.stringify(statusData) | |
| }); | |
| } |
by 結果
では軽く動作確認してみましょう:
良い感じに動いてますね。
実際に社内Slackへ導入しました:
エンジニア同士の交流が弾むきっかけにもなって便利です。
実際に「自分は忙しいから他の人にレビューを依頼してね」オーラを出すことにも成功しているようです。
このカウンターを運用して4ヶ月ほど経過しました。レビュー依頼数は微減といった程度でしたが、レビュー依頼が多いという感覚を定量化できた結果、他のエンジニアと協力しつつ仕事を進め易くなりました。
ピクシブ株式会社では、Slackをhackして日々の仕事を楽しくすることが好きなエンジニアを募集しています。
19年度新卒採用募集要項
中途採用募集要項 / Wantedly
