人間によるコードレビュー(パート2)



これは、コードレビュープロセスでのやり取りと間違いの回避方法に関する記事の第2部です。 ここでは、レビューを終了し、不快な競合を回避する方法について説明します。

基本は最初の部分で概説されているので、 最初から始めることをお勧めします。 しかし、待ちきれない場合は、簡単な要約を以下に示します。優れたレビュアーはバグを検索するだけでなく、同僚がレベルを向上させるのに役立つ正直なフィードバックを提供します。

私の最悪のコードレビュー


私の人生で最悪のコードレビューは元同僚でした。彼女のマロリーと呼ぼう。 彼女は私の数年前に会社で働き始めましたが、つい最近私の部署に移りました。

復習


Malloryがレビューのために最初の変更リストを送信したとき、コードはかなり粗雑でした。 彼女は以前Pythonで書いたことがなく、私がサポートしなければならない不器用なレガシーシステムの上にシステムを開発したことはありませんでした。

私が見つけたすべての問題を忠実に文書化したのは、わずか59ピースです。 私が読んだコードレビュー文献を信じているなら、私は素晴らしい仕事をしました:私は非常に多くのエラーを見つけました。 本当に誇りに思うことがありました。

数日後、マロリーは新しい変更のリストと私のメモへの返信を送信しました。 彼女は最も単純なエラーを修正しました:タイプミス、変数名など。 しかし、彼女は、歪んだ入力データに対する未定義のコード動作や、その関数の1つの制御フロー構造がネストの第6レベルにあるという事実など、高レベルの問題を解決することを拒否しました。 代わりに、彼女は悲しいことに、これらの問題を修正することは開発時間の価値がないと説明しました。

怒ってイライラして、新しいコメントを送りました。 私の口調はプロフェッショナルでしたが、受動的で攻撃的なものに変わりました:「歪んだ入力のために不明確な振る舞いが必要な理由を説明してもらえますか?」マロリーはより頑固になりました。



苦味サイクル


火曜日で、1週間が経過しました。 マロリーと私はまだ同じコードレビューについて手紙をやり取りしていた。 前日の夕方に彼女に最後のコメントを送りました。 彼女が仕事を辞めるまで、私は故意に待っていました。彼女が手紙を受け取ったとき、私は彼女と同じ部屋にいたくなかったからです。

次のレビューを恐れていたので、午前中、私は胃に肉体的な重さを感じました。 昼食から戻ったとき、私はマロリーが職場にいなかったのを見ましたが、新しい変化が彼女から来ました。 私も彼らを受け取ったとき、彼女も部屋にいたくなかったと思いました。

あらゆる反応でりが増すにつれて、私の胸はどんどん強くなりました。 私はすぐに反論キーボードを叩き始めました。彼女は私の提案された変更を受け入れず、同時に彼女のコードが承認されるための言い訳を提供しなかったことを示しました。

この儀式は3週間毎日繰り返されました。 コードはあまり変更されていません。

介入


幸いなことに、私たちの最も経験豊富な同僚であるボブは、この円を壊しました。 彼は長い休暇から戻って、コードレビューでお互いに激しくメモを投げていることに気付きました。 ボブはすぐに状況を行き止まりと評価しました。 彼はレビューを彼に転送するように頼みました。

ボブは、Malloryに新しい変更リストを作成するよう依頼し、検討していない2つの小さなライブラリ(それぞれ30〜50行)を強調表示することから、レビューを開始しました。 マロリーがこれを行ったとき、ボブはすぐに変更を承認しました。

その後、彼はメインの変更ログに戻りました。これは約200行のコードに削減されました。 彼はマロリーが修正したいくつかの小さな発言をした。 次に、変更のリストを承認しました。

ボブのレビューは2日で終了しました。

コミュニケーションの問題


すでにコードが原因で競合が発生していないことを既に推測しているかもしれません。 本当に問題がありましたが、効果的なコミュニケーションのスキルを持っている同僚は明らかに問題を解決できました。

それは不快な経験でしたが、思い出してよかったです。 彼はコードレビューへの私のアプローチを再考し、改善すべき領域を特定しました。

以下では、このような不快な状況を回避するのに役立つテクニックをいくつか紹介します。 後でマロリーに戻り、元のアプローチが逆の効果をもたらした理由と、ボブのアプローチが完全に素晴らしいことが判明した理由を説明します。

技術者


9. コードの品質レベルを1つまたは2つのステップだけ改善するよう努める
10. 繰り返される例に対するフィードバックを制限する
11. レビュー領域を尊重する
12. 大きなレビューを破る機会を探す
13. 心からの称賛
14. 些細な編集が残っている場合、レビューを承認する
15. 事前にデッドロックを回避する

コードの品質レベルを1つまたは2つのステップだけ改善するよう努める


理論上 、同僚はコード内のすべての欠陥を完全に修正したいかもしれませんが、彼の忍耐は無限ではありません。 ラウンドごとに承認を取り始めると、コードを改善する方法についてのすばらしいアイデアが増えているため、彼はすぐにイライラします。

私自身は、学校のように、AからFまでのコードを評価します[6からアメリカの教育システムのステークまで-約 レーン]。 グレードD(3)に対応する変更のリストを取得した場合、著者をグレードCまたはB-に引き上げようとします。 完璧ではありませんが、十分です。

理論的には、品質レベルをDからA +に上げることは可能ですが、これには最大8ラウンドのレビューが必要になる場合があります。 この作業の終わりまでに、著者はあなたを嫌い、あなたにコードを二度と送りたくないでしょう。



「レベルCのコードを承認すれば、コードベース全体がレベルCになることはないでしょうか?」と思うかもしれません。幸いなことに、これは起こりません。 同僚がレベルDからCに昇格するのを手伝うと、彼からの次の変更リストはすでにレベルCから始まっているという結論に達しました。 。

グレードFは、機能的に正しくないか、またはその正確性を確認できないほど混乱させるコード用です。 承認を延期する唯一の理由は、数回のレビューの後、コードがレベルFのままである場合です。 以下の行き止まりに関する章を参照してください。

繰り返される例に対するフィードバックを制限する


作成者が同じ間違いを繰り返し繰り返していることに気付いた場合、各ケースにフラグを立てる必要はありません。 同じことを25回書くのに時間を無駄にしないでください。また、著者は25回繰り返されるコメントを読みたくないでしょう。

このタイプのエラーの2つまたは3つの別々のケースに名前を付けることは正常です。 他のすべてについては、著者にこのタイプのすべてのエラーを修正するように依頼し、特定のケースをリストしないでください。



レビュー領域を尊重する


校閲者が変更のリストからコードのに何かを発見し、それを修正するように作者に依頼した場合の不正なアクションの例があります。 リクエストを満たした場合、通常、レビュー担当者はコードの方が優れていると判断しますが、現在は一貫性がないため、いくつかのマイナーな変更を加える必要があります。 そして、さらにいくつか。 これは、具体的で簡潔な変更のリストが成長し、無関係なものが多数組み込まれるまで続きます。

お腹がすいたマウスがドアに現れたら、クッキーをあげてください。 そして、あなたが与えるならば、彼はコップ一杯のミルクを求めます。 彼は鏡を見て、牛乳の口ひげがないことを確認し、髪を切るためにハサミを求めます...

-ローラジョフェヌメロフ、「マウスクッキーを提供する場合」

: , .

:



- , . — , , - . , , -.

, , , :



, ValidateAndSerialize Serialize. , .

, - , . , , .




~400 , . , . 1000 .



, . , . — . . . , .

, . . 300 , 600 .


, - — .

, , — , . . , , , — .



- , . , -, , :


— , . — , , , .

,


, , . , , .

, :


, , . , . , .

, , . , 5% , . , , . , . 5% , 95% .


- — : , .

, :





. - , . . , .

-


- , - . , -? ?

, , , . , -.


, . , : .

, . , , , , . , ? ? , ? , , .

, . . , . , .


, . , , .


-:


- ? - , ?


. , . , .

, : , . .

. , . , , ? , . , .

, . , . , , .


, , . , . . , , , . - , .

. , , , , . .


, . . , .

. , . .

, : , , . , . , .

. , . , . , . . , , — .

, - .


- — , -. « ». 2002 , , .

Source: https://habr.com/ru/post/J342244/


All Articles