コード記述・Issue管理のルール(hifive開発者向け)
コード記述(主に保守時)の基本的な流れ
- バグや改良すべき点が見つかる
- GitHubにIssueを立てる
- コードを記述する
- 対応するテストコードを記述する
- コミット&プッシュする
- レビュー後、Issueがクローズされ、1つのタスクが完了
ソースコードファイルテンプレート
* Copyright (C) 2014 NS Solutions Corporation
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/
//TODO "(Module Name)"の部分にファイル名(拡張子を除く)を入れる
/* ------ (Module Name) ------ */
(function() {
// =========================================================================
//
// Constants
//
// =========================================================================
//=============================
// Production
//=============================
//TODO エラーコード定数等Minify版(製品利用版)でも必要なものはここに書く
//=============================
// Development Only
//=============================
var fwLogger = h5.log.createLogger(); //TODO カテゴリ名(ファイル名から拡張子を除いたもの)を入れる
/* del begin */
//TODO Minify時にプリプロセッサで削除されるべきものはこの中に書く
/* del end */
// =========================================================================
//
// Cache
//
// =========================================================================
//TODO 高速化のために他で定義されている関数などを変数に入れておく場合はここに書く
// =========================================================================
//
// Dependent Classes
//
// =========================================================================
//TODO 依存しているクラスをここで読み込む
// =========================================================================
//
// Privates
//
// =========================================================================
//=============================
// Variables
//=============================
//TODO モジュールレベルのプライベート変数はここに書く
//=============================
// Functions
//=============================
//TODO モジュールレベルのプライベート関数はここに書く
//関数は関数式ではなく function myFunction(){} のように関数定義で書く
// =========================================================================
//
// Classes
//
// =========================================================================
//=============================
// (Class名)
//=============================
//TODO クラス(newでインスタンス化することを前提としたコンストラクタ関数とそれに関係するprototype等の定義)
//TODO 1クラスごとにクラス名の上記ヘッダコメントをつける
// =========================================================================
//
// Body
//
// =========================================================================
//TODO モジュール本体のコードはここに書く
// =============================
// Code on boot
// =============================
//TODO モジュール読み込み時に即時実行するコードをここに書く
//=============================
// Expose to window
//=============================
//TODO 外部への公開はここでまとめて行う
})();
テンプレートのポイント
- 個々のモジュールでは$を引数に取らない(グローバル側で行う)
- 先頭の「Module Name」はJSのファイル名にする(※ただし拡張子は除く)
- ファイル連結時、どこからどのモジュールかがわかりやすいようにする
- スコープキャッシュなどパフォーマンスへの配慮
- 複数バージョン共存への対応
- 国際化対応、Minify時のコード削除への対応の容易さ
- メッセージの定数化
*モジュラリティ
ビルドによって特定のモジュールのみを含むJSファイルを容易に生成可能
かつ、モジュールファイル単体でも開発・テスト可能
- メッセージの定数化
コード記述時のポイント
※一般的なことを守って記述すればよい。
コミット前には必ずフォーマッタをかける
- diffをとったときに、フォーマットの違いによる(本質的でない)差分が出ないようにするため
必要最小限のコードにする
lengthのキャッシュ、参照頻度の高いプロパティを変数で受けるなど、基本的なパフォーマンス対策は常に行う
対応するテストケースを必ず書く
ソースコード中に使用するコメントタグ
タグ | 意味 |
---|---|
TODO | TODO |
COMPAT | jQueryのバージョンやブラウザの差異吸収など、環境・外部ライブラリの互換性のためのコード箇所に使う。将来古い環境のサポートをやめる場合の参考情報とするため |
コミット時のポイント
コミットは「1つの仕事」の単位で、こまめに行う
- レビュー時に、1つのコミットに複数の仕事(機能追加・バグ修正など)が含まれていると
- 差分が多くなりチェックするのが大変
- どの差分がどの問題への対応なのかの把握が困難
- 後からコミットログを見返すとき、どのコミットにどの変更が含まれているのかが分かりづらい
- 「1つの仕事」の粒度が大きい場合は、複数のコミットに分かれていてもよい(=問題を複数に分割し、順番に対応する)。
コミットの粒度を小さく保つことを常に心がける。
コミットログについて
コミットログの1行目の行頭にはIssue ID(#xx)をつける
- Issue IDをコミットログに入れておくと、GitHub上でIssueを見たときに
対応するコミットイベントが挿入され、トラッキングしやすくなるため - コミットグラフを見たとき、1行目行頭にIDが書いてあるとどのコミットでどのIssueに対応したかを把握しやすい
- 2行目以降だと、一覧表示時にIssue IDが見えない。行の末尾だと、IDの位置が統一されず見づらい
- 必然的に、1つのコミットには1つのIssueだけが含まれるように誘導できるため、コミットの粒度をコントロールできる
コミットログには「変更の理由」や「どのような変更を行ったか」を必ず書く
- どこでどのような変更があったかを追跡するとき、コミットグラフを見てあたりを付けやすくするため
- 「XXの問題を修正」というログしかない場合、実際どのようにして修正したのかをコード差分だけで見なければならず、レビュー時の負荷が高い
- 「なぜ修正したか」という情報は、コードを見てもすぐには判然としない場合もあるため、特にしっかり残しておくべき
- Issueの説明として残す、という手段も適宜併用する
- もし適切でない変更が行われた場合(今はまだその修正をいれてほしくなかった、という場合等)コミットログに変更概要が書いてあればすぐに対応アクションをとることができる
- コミッターは複数のコードを同時に見ているため、各コミットの変更概要がわからないとコードを見るのが大変
GitHub上のIssueの運用
Issueを立てるときのポイント
タイトルだけで、どのようなバグか、またはどのような変更かがわかるように、説明的なタイトルにする
- バージョンアップ時の変更一覧をIssue/Milestoneで管理しているので、
「コード修正」のようなタイトルだと中身を見ないと修正内容がわからず面倒 - 同時に複数のIssueが並行で走っているので、中身がわかりやすいタイトルにしておかないと把握が困難になる
- 後から「あれはどのIssueだったかな?」と見返したいときにわかりやすい
良いタイトルの例:
- 「XXXの後にYYYしたときにcode=12345の例外が発生する」
- 「SSSの場合にTTTがQQQになるように動作を変更」
※基本的に、バグであればそれが発生する状況や発生時に起こる現象を記述、
機能拡張であれば、どのような機能拡張かを記述する。
現象の詳細な説明、なぜその機能拡張が必要かなどの説明は、Issueの説明として記述すればよい。
一度クローズされたIssueについて再度対応が必要になった(ことに気付いた)場合
- 修正コードをコミットする前に、IssueをReopenする
- Issueの履歴上、Reopen ⇒ コミット ⇒ Close という順序にしたい
Issue管理者(Owner)の仕事
- Issueが立てられたら、内容を確認する
- 必要に応じて、補足を求めたり、タイトルをわかりやすいものに変更してもらう(内部Issueの場合)などの初期対応を行う
- 担当者・マイルストーン・ラベルを設定する
- 対応が完了したら、Issueをクローズする(コードレビュー等は適宜行う)
ラベル(タグ)付けの基準
- bug
- いわゆる「バグ」。実装が仕様通りでない、特定のケースで例外が発生する、などの場合に使用
- enhancement
- 機能拡張。APIの追加・変更、引数の追加・変更の場合に使用
- 既存のAPIについて破壊的変更がある場合はspecchangeタグも付ける
- 内部実装だけが変わり、API仕様が変わらない場合はこのタグは使わない。ただし、パフォーマンスの(大幅な)向上など、API仕様は変わらないが明示的に「改良」であると示す場合には使ってよい
- 機能拡張。APIの追加・変更、引数の追加・変更の場合に使用
- specchange
- 仕様変更。既存のAPIについてシグネチャが変更される等破壊的変更がある場合に注意喚起のため使用する
- 既存のAPIには変更がない、もしくは上位互換である(例:引数が後ろに増えるが、渡されない場合は従来通りの仕様で動作)場合には使用しない
- 仕様が変わる場合は、多くの場合enhancementタグも同時につくはず
- wontfix
- このIssueに関して、明示的に「対応しない」ことを示す場合に使用
- 単なる“質問”のIssueの場合にはこのタグは使わない
- この場合はquestionタグを使う
※ドキュメント(JSDoc)の修正、(仕様変更を伴わない)内部実装のみの変更、テストケースの修正など、
フレームワーク利用者からみて機能的な変更がないIssueには、タグをつけない。