私がコードレビューにおいてした誤りと私が今すること

私は最近、コードレビューについてのこれらのつぶやきに遭遇しました:

デビッドK .🎹

ちょうど生命があまりに短いので、彼らがあなたがしただろうものと違ったことをしたので、誰かのPRを拒絶することは思い出させます.
午後13時04分

デビッドK .🎹

✅ 仕事ですか.✅ ビジネスロジック要件を満たすか?✅ テストされ、テストをパスしますか?✅ Linter/formatterはその仕事をしましたか?もしそうならば🚢 それ!
午後13時07分

デビッドK .🎹

しかし、コーディングスタイルのガイドがありますそれが自動化されていないならば(例えば、リンターまたはフォーマッタを通して)、あなたは手動でそれを強制することによって時間を浪費するか、規則があまりに恣意的です.
午後13時09分
それは私が2年前にプル要求を検討している間、私が作ったコメントを思い出させました:

は「someo - random - order - i - think - it - was-正しい」でそれらをグループ化することによって輸入を並べ替えます

後コンマを忘れないでください!

は常にX

を使用します

はY

を決して使わないでください

は、242479182をするのを避けます
あまりにも頻繁に私は過去に私に言った“ベストプラクティス”を複製していた、または私はこの1つのようなブログ記事を読んで学んだと私は盲目的に私はなぜそれをやっていた質問をしなかった後に続いてきた.
しかし、それらのことのどれもバグを修正したり、製品に値を追加したりしませんでした.私は、ちょうど小さなものの上でNitpickによって悪夢を皆の命にしていました.
それらのコメントのほとんどは簡単にeslintを使用して固定することができます.彼らの中には実際には役に立たなかった人もいました.私が違ってそれをするので、それは私の方法がよりよいことを意味しません.デビッドピアノが言ったように、「自動化されていないならば(例えば、リンターまたはフォーマッタを通して)、あなたは手動でそれを強制することによって時間を浪費するか、規則があまりに恣意的です」
私が時間内に戻って、私の古い自己にどんなアドバイスも与えることができるならば、それはそうです:

しかし、何が重要ですか?
これまでの私の経験では、コードレビューは三つのことに役立ちます.
バグ

を避けること
の学習

教育/メンタリング
“Yを使用しない”というような一般的なコメントをするとき、我々はバグを避けていないし、学習や教育の機会としてそれを使用していません.代わりに、現在のケースに対してより具体的でなければなりません.

I think we shouldn’t use Y here because explain your reasoning. It might cause a bug like this problem example. We could fix this by doing solution example.

今何をしますか.
親指の規則として、次のことを思い出します.

私のコメントは、バグを防ぐために支援ですか?そして、教える機会もあります.それで、私は次の質問に答えるようにします.なぜそれが起こるのか?どうやってこれを修正できますか?

このコードは分かりません.それから、それは学習の機会です:「私は、この部分を理解していません.

私はこのコードを理解しています.私はそれが終わり結果を変えないので、単独でそれを去ります、あるいは、私は学習と教育の機会としてそれを使っていますこのように、私は両方とも私のアプローチを教えることができます(他の人がまだそれに精通していない場合)、そして、それを使用しないことの理由を学んでください.

実生活例
しばらく前に、forループ内でasync/waitを使って何百万ものアイテムをフェッチしていたPRに遭遇しました.

for (let post of posts) {
  await doSomething(post.id);
}

私のコードレビューは次のようなものでした.

I think we shouldn’t use this for loop here because it’s blocking the loop. Every post has to wait for the previous request to complete before making another request. This makes the request slower (feel free to run a benchmark and correct me if there’s a mistake in my calculations). We could fix this issue by doing the following:

const batchActions = posts.map(post => doSomething(post.id));
await Promise.all(batchActions);

This way, we're performing those actions concurrently, which results in a faster response. Please, let me know if that makes sense or is there something else I’m missing here.

私のコメントの意図は以下の通りでした.

アプリケーションを傷つける可能性のあるパフォーマンスバグを回避します.

がその問題が起こっている理由を説明します.

私たちがその問題を解決するためにできることの例をあげてください.

私のメトリックが間違っていた場合、または私がそれらの解決策を使用させたいくつかの他の問題を見ていなかった場合に質問されるために開いておく.

持ち帰り
コードレビューのコメントが操作可能であることを確認し、実際に値を製品に追加します.最終的な結果を変更しない硝酸塩の物事に時間を無駄にしないでください.それは予想通りに働きます、そして、それはあなたのCIパイプラインを通過していますか?次に、移動します.
コードレビューは、製品を改善し、開発をスピードアップするためのものです.彼らがあなたを遅くして、チームを傷つけているならば、多分、多分、あなたの実行をチェックする時間です.日の終わりには、“だけでチルアウトと乗り心地をお楽しみください.”😉