Skip to content

レビューされる側の立場で感銘を受けたこと

当時はつらかったけれど、今はレビュー依頼するときの指針としている思い出をまとめる。

サービス設定 > サービスメトリックタブ > メトリック部分を実装より。

正直なところ、(スナップショットを減らすことが困難すぎて膨大になる場合をのぞいて)500行以上変更があるとレビューかなり厳しいです。今回でいえばトップダウンでユーザーに表示しないように作っているので

  • メトリック名変更
  • トグル系ボタン
  • 順序の入れ替え
  • 要素の削除
  • readonly系の対処

くらいは分けられると思っているので、今後はもうちょっと小さくできないか考えて欲しいです。index.tsx、index.test.tsxがdiffを表示できないレベルなのはかなりきつい…

サービス設定画面のサービスメトリックタブを作る話

Section titled “サービス設定画面のサービスメトリックタブを作る話”

グラフ定義の編集フォームより。

このうち編集フォームは、若干の違いはあるけどホストグラフ定義と共用できるので、ここだけ先に作る

この方針はあまり勧められないかなと思いました. というのも, この pr で実装されるコンポーネントを利用するコードがまだないので, コンポーネントが本当に利用側にとって使いやすいものになっているかの判断が難しいです. またそういった状態で実装を作り込みすぎている印象も受けていて, そうなる後々変更したくなったときにつらいことになりそうです. 少なくとも現時点では, 私には方針の良し悪しは判断できませんでした. useFieldArray (や useForm) を使うべきかもしれないしそうでないかもしれない. view / presenter 構成っぽいですがそれがよいのかもわからない. 少なくとも現状の方針で共通コンポーネントとして使うのは難しそうという印象はあります (AngularJS 側で共通化されていない部分まで混ざっている).

まずは先にグラフ定義の一覧部分を作ってから, そこでどのようにコンポーネントを配置するのか や, どのような値を渡すことができるのかなどを明らかにしたいです. その後でこの pr の部分に相当するコンポーネントを, 一気に全ての機能を実装するのではなく, 少しずつ実装されていけるとよいかなと思いました. 概ね shibayu さんのアラート一覧の実装方法 (おそらく現時点で知られているベストプラクティス) をイメージしてもらうと良いかと思います.