Dokumi (日本語)

(English version here)

技術部モバイル基盤グループのヴァンサン(@vincentisambart)です。今日は最近作ったツール「Dokumi」の話をしようと思います。

紹介

他部署のエンジニアの仕事をもっと楽にすることが、技術部の重要な目的の1つです。その中で、Dokumiはモバイル開発者のコードレビューの負荷を減らすためのツールです。

なぜ「毒味」という名前にしたかと言うと、人間がレビューする前に、コードに毒(バグ、不自然なコードなど)が入っているかどうか毒味するツールだからです。別の言葉で言うと、少し進化したCI用のlintツールですね。pull requestが出る度に、Jenkinsがそのpull requestにDokumiをかけます。現在はDokumiはiOSアプリだけに対応してしていますが、今後はAndroidアプリへの対応も考えています。

現時点でDokumiは以下のことをやっています。

  • 静的解析(Xcodeの「Analyze」機能)をかけます。 f:id:vincentisambart:20150603121414p:plain
  • 自動テストを走らせます。 f:id:vincentisambart:20150603121409p:plain
  • コミットされたけどXcodeのバージョンデータしか変わっていないXIB/Storyboardファイルを指摘します。 f:id:vincentisambart:20150603121400p:plain
  • pull requestにmilestoneが指定されていないとそれを指摘します。 f:id:vincentisambart:20150603121419p:plain
  • 静的解析や自動テストを走らせるためにアプリをビルドするので、ビルド中に出ている警告やエラーも同時に拾います。

以上のキャプチャーで見られるように、Dokumiは、GitHub Enterpriseと連動して、指摘をpull requestのコメントとして投稿します。指摘の対象がこのpull requestで変わる行であれば、行にコメントを付けます。対象が変わっていない行であれば、pull requestと関係ありそうなものは普通のコメントにまとめて投稿します。pull requestと関係なさそうなもの(例えばこのpull requestで変わっていないファイルに出る警告)は無視されます。

メリット

XcodeのTestやAnalyzeは、開発者が自分自身でかけることができますが、時間かかるし、かけるのを忘れる時もあります。また、 gitに変更を入れるのを忘れることもあります。なのでDokumiはすべてのpull requestに強制的にかけます。因みにXcodeの静的解析のお陰で人間がレビューしたコードでも不具合や変なコードを見つけることができたのでやってみることをおすすめします。

また、直接GitHub Enterpriseのpull requestにコメントを付けるメリットも大きいと思います。pull requestの度に自動テストを走らせるのは、以前から行われていました。しかし、失敗したテストを知るには、Jenkinsのログのxcodebuildの出力を見るしかなく、簡単ではありませんでした。

実は元々Dokumiの結果はウェブページとして整えようと思っていたけれど、良いデザインが思いつかなかったし、わざわざデザイナーに頼んでもな…と思っていたら、単に、普通の人間のレビューと同じようにすればいいだけだと気づきました。

困ったこと

GitHubのAPIを使って実装しようとした時に、特定の行にコメントを付けるには現時点(2015年6月頭)のAPIでは不足しているところがあると気づきました。

特定の行にコメントを付けるエンドポイントの説明を見ると、指定する必要あるのは「The line index in the diff to comment on.」です。ファイルの行番号ではなくdiffの中の行番号ですね。diffから計算するしかない…そしてこれだけだったらまだしも、そもそもAPIでdiffを取得する方法がない…

GitHubはpull requestのURLに.diffや.patchを付けるとdiffを取得できるのですが、そのdiffはpull requestのコメントに使われているdiffと相違があります:ファイル名変更の対応が違います。.diff/.patchで取得できるdiffはファイル名変更が:

  • 旧ファイル名の削除
  • 新ファイル名の追加

になります。pull requestのコメントで使われているdiffはファイル名変更が:

  • ファイル名変更自体
  • 変わっている行だけの追加・削除

になります。gitをコマンドラインで使うと、git diff--find-renames (略して-M) を付けるかどうかの違いと同じです。

GitHubのAPIで取得できるdiffに find-renames を指定できないならどうすればいいのでしょうか。いまDokumiはレポジトリを手元でcloneして、Ruggedというライブラリを使ってdiffを計算します。もし計算のやり方がGitHubのと違っていれば、ズレが出てくる可能性がありますが、それでも大きな問題にならないでしょう。

pull requestでnew-featuremasterにマージしたいと仮定すると、僕の理解が合っていれば、pull requestのページに出ているdiffをコマンドラインで見たいときは:

$ git diff --find-renames `git merge-base new-feature master`..new-feature

質問コーナー

どの言語で実装されていますか? Rubyですね。僕自身も昔からRubyを使っていますし、社内にはRubyエンジニアが多いからです。

どうやって警告やエラーを拾っているのですか? 主に正規表現を使ってxcodebuildの出力から拾っているだけです。静的解析の指摘はxcodebuild analyzeがビルドディレクトリに生成したplistファイルを見ています。

オープンソース化の予定はありますか? まだ未定です。どの環境でも動くようにするにはコストが高いと思いますし、他人が使えないものをオープンソースする意味もあまり感じないですね。でも実装に関する質問あればお答えします。ソースコードがGitHubにて公開されました

Dokumi (English)

(日本語版はこちらへ)

Let's talk about Dokumi, a tool I have been recently working on.

Introduction

I am part of Cookpad's "Technical Department". One of the department's goals is to make life easier for other engineers. I am in charge of iOS, so I wrote Dokumi to decrease the time that mobile engineers spend on code reviews.

In Japanese, Dokumi means "food tasting", or more literally "poison tasting" (for example, tasting food before a king to make sure the food is not poisoned). I named this tool "Dokumi" because it "tastes" the code to check if any "poison" (bug, strange code) is in it. In other words, it's an advanced lint tool intended to be run by your CI server. Every time a pull request is created or updated, Jenkins runs Dokumi on it. Currently, Dokumi only supports iOS apps, but we are thinking about adding Android support.

Dokumi currently does the following:

  • Runs a static analysis of the code (available in Xcode as "Analyze"). f:id:vincentisambart:20150603121414p:plain
  • Runs your automatic tests. f:id:vincentisambart:20150603121409p:plain
  • Points out the XIB/Storyboard that are changed by the pull request but for which only the Xcode version information changed. f:id:vincentisambart:20150603121400p:plain
  • Points out if the milestone of the pull request has not been set yet. f:id:vincentisambart:20150603121419p:plain
  • To run the static analysis or automatic tests, Dokumi has to build the application, so it also picks up the warnings and errors that occurred during the compilation.

As you can see in the screenshots above, Dokumi posts the issues found as comments to the GitHub Enterprise request. Issues on lines modified by the pull request are added as comments to those specific lines. Issues that are not on a line modified by the pull request but seem to be related are regrouped in one normal comment. Issues that do not seem to be related to the pull request (for example warnings found on a file not modified by the pull request) are ignored.

Merits

Any developer can run Test and Analyze in Xcode by himself. However, it takes time and it is easy to forget to run them. Also it's easy to forget to add some change to git. That is why Dokumi is run on all pull requests. By the way, thanks to static analysis, we did find problems in code that had already be reviewed by humans. So if you never used it, it might be worth a try.

I also think adding remarks directly to GitHub Enterprise's pull requests saves a non-negligible amount of time. Running automatic tests on each pull request was something we had been doing for a while. However, when tests were failing, you had to spend time finding the failure reason in the xcodebuild output inside Jenkins's log.

In fact, at first, I was thinking about putting all the errors in a nice web page, but I could not think of a design I liked, and I did not really want to bother a designer for that. That is where I realized that I could simply do it the same way humans review code.

Obstacles

When I tried implementing that idea using GitHub's API, I realized the current (as of June 2015) version of API makes it difficult to comment on a specific line.

Looking at the documentation for the endpoint to comment on a specific line, you have to pass it "The line index in the diff to comment on.". You first need to map file lines to diff lines. And to do that, first you have to solve an other problem: there is no way to get the diff you need via the GitHub API.

If you add .diff or .patch to a GitHub pull request URL, you can download its diff. However, that diff, and the one used for the pull request's comments, handle file renames differently. In the diff you get by adding .diff/.patch to the pull request URL, a rename becomes:

  • old file name deletion
  • new file name addition

In diffs used for pull request comments, a rename is:

  • file rename itself
  • addition/deletion of only the lines that changed

When using git from the command line, it is the same difference as passing --find-renames (shortened to -M) or not to git diff.

The GitHub API does not let you get the diff it uses for pull request comments, so what can you do? Dokumi currently clones the repository locally, and using the Rugged library, it generates its own version of the diff for the pull request. Of course if the way the diff is generated is different from the way GitHub does it, the line the comment ends up on might be incorrect, but even it this happens it should not be a big problem.

If a pull request is to merge new-feature into master, the way to display its diff from the command line is the following:

$ git diff --find-renames `git merge-base new-feature master`..new-feature

Q&A

In what language was Dokumi implemented in? Ruby. I have been programing in Ruby for a while, and at Cookpad we have many Ruby engineers, making it a good fit.

How do you detect warnings and errors? Mainly using regular expressions on the output of xcodebuild. For static analysis, the plist files generated by Xcode Analyze are also used.

Do you have any plan to open-source it? Not decided yet. It would be costly to make it work on most environments, and I do not feel it is really useful to open-source something that most people could not use easily. But I would be happy to answer any question about its inner workings. The source code has been made public on GitHub.

チーム開発の進め方

f:id:Yoshiori:20150604175344p:plain

こんにちは!クックパッド編集室メディア開発グループ長の @yoshiori です。

今回はウチのチームの開発の進め方や見積もりの仕方を説明しようと思います。

実はコレ系の話は 5 年前にもデブサミで発表 したのですがこの時はリリースまで 1 年とかのレベルのプロジェクトの進め方の話でした。今回は 1,2 ヶ月でリリースまで持っていく開発の進め方を説明します。

動画サービス部分を microservices 化するときに実際に行った事を元に説明します。開発者は 3 人で 1.5 ヶ月位の開発です。

何故このようなことを行うのか

誰だって楽しく仕事がしたいし、なるべく不安などは無い方が良いはずです。 例えば自分がやっている作業がどうなったら終わりなのかわかっていなければ不安でしょうし、いつまでに作ればいいのかわかっていなければ不安でしょう。

そういった不安をなるべく無くすためにうちのチームでは 見える化コミュニケーション を重視し、メンバー全員で楽しく開発することを目標にしています。

まずは作らなきゃいけないものの洗い出し

いわゆるタスク出しってやつですね。正直この作業はあんまり面白いものではないです。特に開発初期などはエンジニアはまっさらな状態からコードをドンドン書いていけて凄く楽しいのでいきなり手を付けたくなります。ですがチーム開発でそれをやってしまうと比較的早く破綻しますし、その時にはメンバーそれぞれのモチベーションにも差が出てきたりして軌道修正をするのが難しくなります。なのでメンバーの意識も高い初期になるべく楽しく行うのが大事だと思います。

まずは全員で時間を作りやらなきゃいけないタスクの洗い出しをします。 1,2 ヶ月で完成させるようなものはアジャイル開発でよく使われているユーザーストーリーで分けると粒度が少し大きくなってしまうため機能単位でタスク出しをしています。

どの程度の粒度で出すかというと

  • DB 設計
  • Video テーブル移行スクリプト
  • Video 取得内部 API

のような感じで出しています。 実際に全員で「もう出すもの無いよね」というところまで出し切ったらそれを一個づつ付箋に書いていきます。

出したタスクの重さを決める

f:id:Yoshiori:20150604175437p:plain

次に出したタスクの重さを決めていきます。ユーザーストーリーで分けているときはストーリーポイントとか言われているものですね。コレはなるべく抽象化したポイントであらわし、かかる時間などの具体的な数値で出さないことが大事です。以下説明のためにこの数値を SP (ストーリーポイント) と書きます。

実際の数値の出し方ですがプランニングポーカーと呼ばれている手法を使って行います。簡単にうちのチームで行っているプランニングポーカーの説明をします。

  1. 「1,2,3,5,8,13,∞,?」 どこかで見た数列 + 無限と ? のカードを人数分用意します。
  2. すべてのタスクの中からちょうど真ん中くらいの難易度だろうと思うのもをみんなで選びその SP を 5 にします。
  3. まだ SP を決めていないタスクを選びます。
  4. 簡単にそのタスクの機能の説明をします。
  5. それぞれがカードの中から SP を選び同時に出します。
    • 最初に出したタスクが 5 だというのを参考に考えます。
    • 専門分野など情報が不足しすぎていて自分には見積もりできない時は ? を出します
      • これはその後説明を求めます。
    • タスクが大きすぎて 13 を大幅に超えると思ったら ∞ カードを出します
      • タスクをさらに分割します
  6. 数値が合わなかったら一番大きい数字と小さい数字を出した人間がそれぞれの根拠を説明します。
  7. 6 で出た意見を参考にもう一度全員で SP を出します。
  8. 全員の数字が揃うまで 6,7 を繰り返します。
  9. すべてのタスクに SP が振られるまで 3 〜 8 を繰り返します。

これですべてのタスクに SP が振り分けられます。 プランニングポーカーは面倒臭い見積もりをゲーム感覚で楽しく行えるのでオススメです。

イテレーション期間

次にイテレーション期間を決めますが、もうコレは色々経験してきた結果

水曜日始まりの 1 週間を 1 イテレーションとする

が一番しっくり来るので僕の独断でそうしています。簡単に理由を説明すると

  • 1 週間より短いと細かすぎて朝会などと区別があやふやになる
  • 1,2ヶ月の開発に 2 週間だと 2 〜 4 イテレーションしか回せずうまくリズムにならない
  • 月曜日で振り返りを行うと先週の問題点があんまり気にならなくなってる
  • 金曜日に振り返りを行うと月曜日には改善しようと思ってたことを忘れる

という理由からそうしています。

スケジュールぎめ

1 週間というタイムボックスが決まったので、それぞれのイテレーション期間でどのタスクを行うかを決めていきます。 最初はだいたいザックリ「今週はこのくらい出来るんじゃね?」という量を割り当てていき、次の週などもそれを元に割り振っていきます。この時に大事なのはタスクの順番を意識することです。例えば最初に例に上げた「DB 設計」を終わらせないと「Video テーブル移行スクリプト」は実装できません。こういった順番を考慮しつつ各イテレーションに作業を割り振って行きます。(後で説明しますがココで上げたスケジュールは大体破綻しますw)

実際に開発を回していく

f:id:Yoshiori:20150604175516p:plain

スケジュールも決まったので実際に開発にかかります。うちのチームでは基本的に個々の作業管理はツールとしてのかんばん で行っていますが少しアレンジしています。

  1. タスク自体はイテレーションを表した大きめの紙に貼られている
  2. 自分が実際に作業しているものには自分の名前の書かれたマグネットを置く
  3. 終わったものには猫のシールを貼る
  4. 予定になかったタスクが発生したら赤い付箋に書き、そこにも SP をふる
    • 突然降ってきた仕事
    • 仕様考慮漏れ
    • バグ修正

タスクが終わったものはシールを貼って表していますがコレはその作業が完全に完了するまで行わないことを徹底しています。(アジャイルで「完全 Done 」と呼ばれているものですね)

振り返りを行う

水曜日には振り返りを行います。まず計画で上げたタスクで完全に完了したものの SP をすべて合計して算出します。 コレがチームの 1 週間で行える作業量という指針になります。ついつい「この 8SP の作業、あとちょっとで終わるから」と言って合計数に含めたくなりますがやめましょう。

さて、だいたい最初のイテレーションが終わるとすでに最初の計画が破綻していると思います。 今回うちのチームも 1 イテレーションで 30SP 位の作業は出来るだろうと見込んでいたのですが、 20SP しか完了出来ませんでした。つまり 10SP の作業はあぶれ、今後のイテレーションに割り振られている 30SP をすべて 20SP に修正すると一気に 1.5 倍ほどのスケジュールになってしまいます。

ここで大事なのが振り返りです。

まずは我々は1イテレーションに 20SP しか消化できないという認識をします。 そこで何故 20SP しか消化出来なかったのかなどを話し合います。今回は 3 人なので KPT のような比較的きっちりした振り返りではなくスタンディングでお互いの意見を言い合う形で行いました。 最初のイテレーションはデータ移行などのタスクに引きづられ、大きめなタスク( 13SP )が実際にデータを入れてみたら修正する箇所が出てきたりして完全 Done にならなかった事がわかりました。 幸いにもその修正はすぐに終わるだろうという事で今回はスケジュールの見直しはしないで行けそうだということになりました。

ここで厳密に今後のスケジュールを見直すことも可能ですが、いきなりここで厳密にやってもテンションが下がるだけなので行いませんでした。実際、本当に持ち直せなかったら次のイテレーションでスケジュールの切り直しをするつもりでしたが、うまく持ち直したのでそのまま進みました。

イテレーションを回していく

上記のようにイテレーションを回しながら

  • 抜けているタスクはないか
  • タスクに割り振られている SP は妥当か
  • イテレーションに割り振られているタスク量は適切か
  • スケジュールを切り直す必要はないか

は話し合っていき、スケジュールをドンドン正確にしていきます。

チームのパフォーマンスチューニング

f:id:Yoshiori:20150604175552p:plain

このようにイテレーションを回していくとベロシティ(チームの平均消化 SP)がわかってきます。あくまで指針としてですがこのような数値を毎週出しておくとチームの健康状態を図れます。ベロシティが落ちていればなにか問題があるのでしょう、逆に上がっていれば何か改善する事があったのでしょう。

振り返りの時に「なんか今週仕事があんまり進まなかった気がするよね!なんでだろ?」と感覚的にいうよりも「何故今回はベロシティ下がったんだろう?」というほうが数字にも出てるのでチームメンバーも共感でき、一緒に改善案を考えれます。

パフォーマンスチューニングの基本「推測するな、計測せよ」です。

まとめ

チーム開発の進め方として主にタスク出しやスケジュールの切り方を説明してみました。僕はもともと「スケジュール立てるのとか苦手だし、誰か得意な人がやればいいんじゃね?」とか思っていましたが今では

タスク出しやスケジュール立てるのは才能ではなく技能なのでやり方覚えれば誰でも出来る

と感じています。実際にやっていることはプログラムを書くときと同じで大きい問題を小さく切り分けて順番に対処していっているだけです。そして計測してそこから逆算しているだけです。

やり方を覚えて身に付ければ誰にでも出来る事なので、まだ習得していない人や昔の僕の考えと同じようなことを考えている人は早めに習得しちゃうことをオススメします。

アジャイルな見積りと計画づくり ~価値あるソフトウェアを育てる概念と技法~

アジャイルな見積りと計画づくり ~価値あるソフトウェアを育てる概念と技法~

  • 作者: Mike Cohn,マイクコーン,安井力,角谷信太郎
  • 出版社/メーカー: 毎日コミュニケーションズ
  • 発売日: 2009/01/29
  • メディア: 単行本(ソフトカバー)
  • 購入: 74人 クリック: 764回
  • この商品を含むブログ (225件) を見る