SKIYAKIの開発ガイドラインについて
はじめに
こんにちは。
SKIYAKIのエンジニアの家(@ie_webengineer)です。
SKIYAKIでは長年、開発する際のガイドラインがなく
チームでなんとなくルールが定まっていたというイメージでしたが
暗黙の了解になっていることも多く、独自ルールが多すぎて新しく入社する方が戸惑いを覚えたり、
プロジェクトごとのチームでレビュワーは最低何人いるよねとかも違っていたため、それぞれのルールで違和感を感じることが多くなり、昨年ルールを明確に制定しました。
2024年に対応した話で直近ではあるものの
今更感ある話なのですが、共有することで誰かのためになるかもしれないので共有してみます。
開発ガイドラインとはなんぞや
主に以下が議論対象となりました。
- PRレビュイーの心得
- PRレビュアーの心得
- コーディングガイドライン
ルールを作るにあたってどうしたか
- 強制参加ではない週1回の開発改善MTGを実施
- 目的に応じて誰が何やるかを決定
- 必要に応じてルールを決めるため、挙手制 + 確認が必要なメンバーを招集し、議論(notionにまとめる)
- 開発全体定例MTGにて確定したルールを共有
- PRテンプレートに転記
結果的に設定したルール
レビュイーの心得
・コードの変更は小さくしてPRを作成すること
→ 膨大なファイル数になる場合(新機能等)は小さい変更ごとにブランチを分けることを推奨
・基本的にはセルフレビューを行うこと
・PRの実装説明はできるだけ背景、意図、確認してもらいたいことなど書く
→ whatだけはNG(「tableにカラムを追加しました」だけ等)、レビュワーの負荷を上げないため
・アサインしたレビュアー全員にapproveもらってからリリース
→ リリース日が決まっている場合はレビュー締め切り日(遅い/早いに関わらず)を書く
→ PMによる仕様の確認等がある場合は、それは必ず行う
→ 不安なら多く人や詳しい人の承認を待っても良いし問題なさそうと考えているなら1人でも構わない。(期限の都合やレビュアーが足りて不要になった場合はレビュー中の可能性もあるため外す旨を相談の上レビュアーから外す)
※ 一度でもレビューコメントするとレビュアーから外せないのでレビュアー外せない場合は担当レビュアーのapprove不要の許可コメントを残す
→ レビュー時必要な修正ではあるがリリース最優先事項ではない場合は課題化(backlog化等)してコメントにリンクを貼る
→ しかしリリース行為の責任は持つ
・全てGitHub(PR)に閉じる必要はないがslackや口頭で話したものは最低限記載(linkをはる)する。
→ そのトピックに無関係な人が見てもわかるようにする
・コミットメッセージは「レビュー対応」「修正」「リファクタ」など何をしたか具体的でないコミットメッセージは禁止
→ 具体的に何をしたかわかるメッセージを心がける
レビュアーの心得
・レビュワーのアサインから3営業日を目処に確認すること(もちろん早いほうが良い)
・原則、コードレビューでリリースを遅らせてはいけない
・柔らかい文章は良いが、それがかえって「どうアクションしたら良いか」わからなくなっていないか
→ 自分ならどうするか具体的に書いておくのが良い
・以下の分類でコメントを書く
MUST: 要修正
IMO : 相談した上でOK/NGを決めたい
nits: リリース余裕があれば直して
Q : 質問 コードの意図や、仕様の確認など
・レビューコメントは放置せず、済んだものはresolveしてあげる
・アサインされてるが、忙しくて見れない場合slack報告
コーディングガイドライン(当たり前のもの含む)
・よく言われる基本的なコーディングルールを遵守する(書き出すとキリがないので以下参考)
→ 参考: https://zenn.dev/mbao/articles/my_idea_of_the_best_coding_rules
・セキュリティに関する考慮
→ 参考: https://www.ipa.go.jp/security/vuln/websecurity/about.html
1. 個人情報系データの暗号化対応
2. APIKeyなどのハードコーディングをしない
3. SQLインジェクション攻撃対策
・パフォーマンスに関する考慮
1. N+1対応
2. 適切にDBにINDEXをはる対応
3. 画像のリサイズ
・各サービス/機能の影響を意識する
1. 他プロダクト/他機能への影響
2. 管理画面の対応
3. 翻訳対応
4. 二重クリック(二重タップ)の対策がされているか
5. 独自ドメインサイトへの考慮
6. カラーテーマの変更への考慮
7. アプリ ⇄ Webの対応
8. 例外処理の対応
9. エラーメッセージの配慮
10. APIドキュメントの対応
11. プッシュ通知の対応
12. 退会済みユーザーの影響
さいごに
すでにいるメンバーにとっては当たり前になっているルールなどは、
自発的に進めないと特に進行されず忘れ去られてしまったりする(よくある話)ので、
こういう会社として決めておきたいことに少しでも興味あるメンバーを巻き込んでルールを制定する会議を設けてそこで色々議論し、ルールを確定させました。
テーマは 思いやりの精神 って感じです。(組織で仕事する上で一番大事ですよね!)
「こうだと思ってました」をなくすために開発部全体に決まった内容を共有し、展開しました。
それでも足りないと思うので、それをPRテンプレートに転記し、いつでも気軽に見れるようにしました。
開発部のMTGではPMではない作業者の共有は少ないため、
結果的に作業者目線のルール改善などの場を広げるきっかけになったためにやって良かったなと思います。
参考になった本
Looks Good To Me
Clean Code アジャイルソフトウェア達人の技
達人プログラマー-第2版-熟達に向けたあなたの旅-
株式会社SKIYAKIのテックブログです。ファンクラブプラットフォームBitfanの開発・運用にまつわる知見や調べたことなどを発信します。主な技術スタックは Ruby on Rails / React / AWS / Swift / Kotlin などです。 recruit.skiyaki.com/
Discussion