OSSの開発モデルを、そのまま社内に持ち込むのは止めたほうがいい(もしくはコードレビューの話)

以前以下のようにツイートした話だけど、OSSの開発モデルを社内開発にそのまま持ち込むのは危険。

チーム開発の場合、簡単にコメント返せないようなPR送られた時点で負けだと思ってる。無駄なコードが生産されないよう事前に設計を詰める作業をすべき / “雑なレビュー - ✘╹◡╹✘” http://htn.to/by5or1

https://twitter.com/kazuho/status/431602421068877824


今日一般的に想定される「OSSの開発モデル」はEric S. Raymondの有名なエッセーである「伽藍とバザール」によって解説されたところの「バザール」である。

「バザール」モデルにおいては、多くの改善提案の中から最善と考えられる実装が取り入れられ、コミュニティで共有されるようになる。同エッセーから引用するなら、

 ふりかえってみると、Linux の手法や成功の前例は GNU Emacs の Lisp ライブラリと Lisp コードアーカイブの開発にみることができる。Emacs の C のコア部分やその他 FSF ツールみたいな、伽藍建築方式にくらべると、Lisp コードのプールの進化は流動的で、すごくユーザ主導で行われた。アイデアやプロトタイプ・モードは、安定した最終形に落ち着くまで 3 回も 4 回も書き直されるのがしょっちゅうだった。

今日的に言い換えるなら、あまたあるブランチやpull-requestの中から最良のものが選択されるということ。

これは、エンジニアリングリソースが潤沢にあるからこそ採ることができるアプローチ。そして、成功したOSSは必ずエンジニアリングリソースが潤沢になるので(成功したOSSの場合、問題があったら直そうとする人が多数いるから)、OSSの開発モデルとしては「バザール」モデルがうまく機能するように見える。

しかし、社内開発においては、限られたエンジニアリングリソースをできるだけ効率よく活用することが求められる。別の言い方をすると、エンジニアが「無駄な作業」をする可能性を減らすことを考えるべきだが、これはバザールモデルとは相反する。


コードレビューの話はその具体例であって、社内でのグループ開発においてプルリクエストの評価(やコードレビュー)で設計の問題を指摘するというアプローチは、実装完了後に設計の評価を行うことになるという意味で、手直しが発生した場合の損失が大きくなりがち。

それよりも、実装開始前に設計レビューをやるなどして、無駄なコーディング作業を減らすことを考えるべき。


参考:
テストファーストなGitワークフローについて - kazuhoのメモ置き場
https://twitter.com/t_wada/status/443945356665962496のスレ

MacBookのCPUクロックを表示する方法(Intel Power Gadget)

Intel Power Gadgetをインストールすると、手元のMacのCPUクロックやGPUクロックを表示することができる。以下は同ツールをMacBook Pro(Early 2013; 2.4GHz Core i7)で実行している画面だが、負荷をかけた際に定格の2.4GHzを上回る3.2GHzで動作していることが分かる。

定格より速い速度で動いてるのを見れるのもうれしいし、アプリ全部終了しても高いクロック数(=高い消費電力)に張り付いていたりする場合は、再起動した方がバッテリの持ちが良くなるから、こういうツールをいれておくと確認できて便利。

WebSocketsでの送信処理に関する注意点(CLOSINGステートとcloseイベントに関する疑問)

WebSocketsの接続状態には、CONNECTING / OPEN / CLOSING / CLOSEDのステートが定められている。

一方、CLOSING状態への遷移に対応するイベントは存在せず、CLOSE状態へ遷移した際にcloseイベントが発生するとされている。

つまり、「closeイベントが来るまでは送信できるぜー」ってなコードを書いてると、正常系なのにsend()で例外発生する可能性がある。

なので、面倒だけど、

if (ws.readyState == 1) {
    ws.send(...);
}

のようなガードを入れる必要がある*1。もしくは、ws.readyState == CLOSING 状態になったらcloseイベントのハンドラを呼び出すようなラッパーを書く必要がある。

と、以上の結論に至ったんですが、あってますでしょうか? >識者


以下、規格からの引用。

CONNECTING (numeric value 0)
  The connection has not yet been established.
OPEN (numeric value 1)
  The WebSocket connection is established and communication is possible.
CLOSING (numeric value 2)
  The connection is going through the closing handshake, or the close() method has been invoked.
CLOSED (numeric value 3)
  The connection has been closed or could not be opened.

When the WebSocket closing handshake is started, the user agent must queue a task to change the readyState attribute's value to CLOSING (2). (If the close() method was called, the readyState attribute's value will already be set to CLOSING (2) when this task runs.)

When the WebSocket connection is closed, possibly cleanly, the user agent must queue a task to run the following substeps:

  1. Change the readyState attribute's value to CLOSED (3).
  (略)
  3. Create an event that uses the CloseEvent interface, with the event type close, ...

*1:全二重なプロトコルなので、else節は不要で問題ないはず

DRY(don't repeat yourself)するかしないか、その判断基準について

「過剰なDRYが技術的負債を生む」みたいな内容の記事を書きたいが、うまく言語化できない。「過剰な食事制限が健康を損なう」程度の内容に成り下がりそうだけど、そんなんじゃないんだよ…
@methane 実装におけるDRYみたいなものを考えていて、そうすると前者のDRYというのがどこに位置づけられるかはわからないんですが、とにかく暗黙知みたいなものを過剰に増やすDRYは良くないよね、というような話なんです

という@moriyoshitさんのツイート(1, 2)を見かけたので、僕の考え方をコメント。moriyoshitさんの考えたい問題とは、ずれてるかも。


DRY化の功罪とは何か?

僕の理解で言うと、共通するコード片をDRY化することには以下の変化をもたらす。

  • 循環的複雑度は変化しない
  • コールグラフは複雑化する
    • モジュールをまたぐDRY化を行うと、モジュール間の依存関係も複雑化する*1
  • 関数内の複雑度は低下する

過剰なDRY化が行われる背景としては、第2点の議論が見落とされがちなんじゃないか、と思ってる。


ではどうすべきか?

上記の3点より、

  • 関数内の複雑度の低下によるメリットが、コールグラフの複雑度の上昇によるデメリットを上回る場合のみ、DRY化を行う
  • モジュール間の複雑性上昇を抑えるため、DRY化はできるだけ局所的に行う*2

を基本的な指針にしています。

*1:もしくはより結合度が高まる

*2:つまり、まずは関数内での子関数定義であったり、モジュール内に閉じた関数としてDRY化を実装することを考える

クローズドソースなnode.jsライブラリの依存関係をdedupeする話

node.jsで開発してると、社内で作ってるライブラリの依存関係がごちゃごちゃしてくることがありますよね。

app --+--> libA ----> libB
      |
      +--> libB

みたいな。こういうときlibBがnpmに登録されたモジュールであればnpm dedupeコマンド一発でlibBをひとまとめにしてくれるけど、gitに登録されてやつだとしてくれない(のは以前も書いた)。で、まあやっぱり不便だし、代替策があるかどうかも良くわからんので、ぱぱっと書いた。

force-dedupe-git-modules - npm

このコマンド使うと、github.comのプライベートレポジトリやGitHub Enterpriseに置いてあるnpm形式の社内ライブラリについて、強制的にdedupeかけることができる。

簡単。

トピックブランチ内の変更を無視してgit bisectする方法

merge commitについては1st parentのみをたどりながらgit bisectしたいことってありますよね? (参照: テストファーストなGitワークフローについて - kazuhoのメモ置き場)

そういうとき、1st parent以外に属するcommitをgit bisect skipするの、どうやったらきれいに書けるのかなと思ってたのですが、以下のやりかたが一番よさげ。

$ git bisect skip $(comm -23 <(git rev-list HEAD | sort) <(git rev-list --first-parent HEAD | sort))

thanks to: ひろせ31 on Twitter: "@kazuho comm -2 -3 <(cat A|sort) <(cat B|sort) とか?", Masakazu Takahashi on Twitter: "@kazuho 訂正。comm -23 A B でした"