"ذات مرة أجرينا مراجعة للكود ، وكتبنا التعليقات في البريد بأرقام الأسطر. كانت مضحكة جدا. من المحترفين: لم ينظر أحد إلى أي شيء على الاختلافات ، في IDE. ولكن كان هناك أيضًا ناقص: بعد بعض الدمج ، تغيرت أرقام الأسطر ".
الكسندر ماكاروف ، يي
"شركتنا لديها مفهوم مثير للاهتمام - طلب كرسي . هذا عندما ، داخل نفس المكتب ، يلتف إليك مطور على كرسي ويقول: "انظر ، هذا أسرع من إنشاء طلب سحب."
انطون موريف ، WormSoft
في الآونة الأخيرة ، كان هناك تسجيل عام لبودكاست SDCast حول مراجعة الكود على YouTube. لقد اخترنا وفككنا أكثر الأشياء إثارة للاهتمام من هذه القضية.
نسخة صوتية كاملة على Spotify و Ya Music وكملف لتنزيل
نسخة فيديو كاملة على Youtube
لا تتعامل مع مراجعات الكود مثل التحقق من شيء ما أو البحث عن الأخطاء
سيرجي جوك ، Skyeng: أعتقد أن الهدف الأساسي لمراجعة الكود هو مشاركة المعرفة داخل الفريق وإيجاد الحل الأفضل. يراجع الفريق التغييرات المقترحة - ويتم توزيع المعرفة حول المجال بالتساوي بين أعضائه. يفهم مؤلف الحل كيف يمكن تحسين الكود الذي يعتقد أنه مثالي.
لذلك ، مراجعات الكود ليست شيئًا يجب تجنبه أو القيام به بشكل أسرع. هذا استثمار في العمل والفريق: الكود يتحسن ، الفريق يتحسن. نعم ، في البداية لم يعجبني عندما تم تغليف الطلب (ومن يرغب في ذلك).
لكن بعد ذلك بدأت أعتبرها عملية تعليمية ، مثل قراءة الكتب أو المشاركة في المؤتمرات.
شعرت بهذا بالإضافة إلى نفسي ، بصفتي مطورًا نشأت مع هذا النهج.
لكن هناك فارق بسيط. من المؤكد أن العديد منكم قد صادف طلبات لألف سطر ، حيث تم خلط إعادة البناء ، والميزة ، وبعض التغييرات الطفيفة. من خلال خلط أشياء مختلفة في طلب واحد ، فإننا نعقد كل من مشاركة المعرفة وحياة المراجع: سيكون من الصعب عليه تقييم ما إذا كان سلوك النظام قد تغير ، وما إذا كان قد تم استيفاء متطلبات العمل ، وما إذا كانت المشكلة ككل قد تم حلها - وهل الحل ناجح؟
لاحظنا هذه اللحظة في فريقنا وقدمنا قاعدة: في طلب سحب واحد ، لا تتدخل في تغييرات ذات طبيعة مختلفة. أنت ترسل إعادة البناء بشكل منفصل ، ميزة منفصلة وتغييرات طفيفة منفصلة.
تقرير سيرجي عن ممارسات فريقه. النسخة النصية هنا .
تتم عادةً مراجعة هذه الطلبات بسرعة وسهولة ، ومن المرجح أن تتلقى تعليقات جيدة. ومن جانب المشرف ، هناك إيجابيات: إذا كنت تحب إعادة البناء ، والميزة التي بها خطأ ، فيمكنك دمج إعادة البناء بشكل منفصل.
ألكساندر ماكاروف: أوافق على أنه لا ينبغي عليك قضاء أقل وقت ممكن في مراجعات الكود. إذا تم فتح الأقواس ، فإن هذا الرمز يقوم بشيء ما - فهو لا يعمل بهذه الطريقة. إذا لم يستطع المراجع أن يضمن الكود حتى النهاية ، فهذا يعني أن مراجعة الكود لم يتم تنفيذها.
لذلك ، بدأنا الآن في تجميع عدد كبير نسبيًا من الإرشادات لأنفسنا ، ويتحدث أحدهم عن مراجعة الكود.
جزء من فريق Yii في التوجيهي .
لكن بالنسبة للأطروحة المتعلقة بطلبات السحب الصغيرة المنفصلة: كانت لدي مواقف جاء فيها قائد وقدم شيئًا من هذا القبيل. كان الفريق معاديًا. كيف حصلت عليها؟
سيرجي جوك: كان هناك فريق بالتوازي نتفاعل معه ، استخدمنا واجهة برمجة التطبيقات الخاصة بهم. لقد صنعوا مجموعة من الميزات في شهر واحد ، فعلناها قليلاً. أي أننا في البداية لم نركز على تقديم الميزات ، ولكن على الجودة مع هذا النهج. واتفقنا مع الشركة على أننا سنصدرها بشكل أبطأ ، لكننا نحاول عدم كسر أي شيء. بعد شهر ، انهار أحد الجيران ، ثم انهار آخر. لكننا لا نفعل. وفي هذا المثال ، فهم الجميع كل شيء.
كونستانتين بوركاليف ، SDCast:كانت لدي عمليات لتنفيذ مراجعات الكود بشكل عام - ولم يكن الأمر سهلاً ، على الرغم من أن الجميع بدا وكأنهم يفهمون أنها كانت جيدة ، نعم. أنت تقول ، "يا رفاق ، دعنا ندمج من خلال طلب سحب." يقولون لك: نعم ، هناك ميزة صغيرة.
يعمل المبدأ هنا جيدًا أنه عندما ينكسر شيء ما ، فأنت تقول: "ولكن إذا قدمت طلبًا ، فسننظر إليه وببساطة لم يأتِ إلى ذلك". الفكرة هي أن يفهم الناس الأخطاء من تجربتهم الخاصة. محاولة الفرض لا تنجح دائمًا.
مدى سرعة المراجعة
خلال البث ، تم التصويت بين الجمهور. هنا هو واحد.
كونستانتين بوركاليف: غالبًا ما يكون شهر يونيو على هذا النحو: "حسنًا ، لقد شاهدت طلبي ، هل هو جيد هناك؟ كتبت ذلك ، وشدّت قبضتي وانتظر ".
ولدي مهمتي الخاصة ، لا يمكنني الوصول إليها إلا في المساء ، أو لا أعرف على الإطلاق ...
سيرجي جوك: في فريقنا ، كانت المراجعة دائمًا أولوية. لذلك ، هناك لائحة - عندما يصل طلب سحب ، فإنك تنهي ما تفعله الآن ، حتى لا تفقد السياق ، وتذهب للمراجعة. لأن ما أنت على وشك القيام به لا يزال قيد التنفيذ. وقد تم بالفعل إنجاز هذه المهمة.
تمت كتابة الكود بالفعل ، ويبقى فقط البحث عنه ودمجه وتحميله إلى المنتج.
وهذا يعني أنني أنهيت شيئًا خاصًا بي ، وتحولت ، وبسرعة نظرت واستمرت في العمل. ربما ، 3 مرات في اليوم يصرف انتباهي عن مهامي للمراجعة. لكن ، مرة أخرى ، عليك أن تفهم أن كل شيء في فريقي ينقسم إلى طلبات سحب صغيرة ، 200-300 سطر من التعليمات البرمجية لكل منها. وفقًا لذلك ، يتم إنفاق الحد الأدنى من الوقت.
"من يراجع أقل أهمية من من"
قسطنطين بوركاليف: هذا يطرح السؤال - من الذي يجب أن يراجع. يؤدي فقط؟ فقط السينور؟ أو هل يمكن ويجب أن يُعطى لشخص أقل كفاءة ، فقط حتى يحاول الشخص أن ينمو؟
وعندما سئل الناس عما يجب مراجعته ، أجاب الناس بهذه الطريقة.
ألكساندر ماكاروف: إذا كان لديك الكثير من الأشخاص في فريقك ، وكان القائد قد خلق عنق زجاجة من نفسه ، فإنه يبطئ الفريق بأكمله ونتيجة لذلك يجعل الفريق أسوأ بكثير. كقائد ، عندما كنت أعمل في Skyeng ، كان لدي 15 طلب سحب يوميًا في ذروتها ، وليس أصغرها. لقد خصصت وقتًا للمراجعة في الصباح والمساء.
من الضروري مراجعة الجميع. ربما باستثناء القصص التي يكون فيها "النار ، كل شيء يشتعل" - لن يزداد الأمر سوءًا هناك.
لقد أخطأت ، هذا أمر طبيعي - على الرغم من أنني كنت أستخدم نفس المشروع لسنوات عديدة. لذلك ، أحاول الآن دعوة أكبر عدد ممكن من الأشخاص لمشاهدة طلبي. هذا جيد ويجب أن يكون.
إنها مسألة أخرى ما إذا كان يجب على الجميع الوثوق بالمراجعة. كان لدي رجال يمكن أن يكتشفوا شيئًا رائعًا ، لكن لديهم مشاكل كبيرة في التركيز: على سبيل المثال ، قضى أحدهم 6 ساعات في المراجعة. كان علي تعليم الناس كيفية إدارة وقتهم.
إذا كنا نتحدث عن شركات تجارية ، فأنا أؤيد تحديد من يتحمل هذه المسؤولية ومن يمكنه المراجعة حسب الرغبة - وكم من الوقت يمكنك أن تقضيه في المراجعة يوميًا ، اعتمادًا على هذه الحالة.
انطون موريف:كما أرى ، هناك عمليات مختلفة. إذا كنا نقوم ببعض الميزات التي يجب إصدارها في وقت قصير ، فلن نسمح لـ June برؤية الرمز. ولكن إذا كنا نقدم نوعًا من المنتجات الداخلية أو كان الموعد النهائي غير مرتفع ، نعم ، إنها ممارسة رائعة للسماح لـ يونيو بمراجعة ما فعله المطور الرئيسي أو كبير المطورين. لذلك سوف يفهم Juns بشكل أفضل ما يحدث في المشروع ككل وكيف تعمل آلية صنع القرار.
"هذا رفض على الفور"
سيرجي جوك: نفذت Skyeng فحصًا لرسالة الالتزام: يجب أن يكون هناك رقم مهمة في JIRA ، وإلا فلا يمكن دمج طلب السحب. في بعض الأحيان ، تفتح الرمز ، ولا تفهم ما هو موجود - تفتح مهمة ويمكنك فهم شيء ما.
بالنسبة للباقي ، في البداية كان الأمر صعبًا بالنسبة لنا ، ثم قررنا رفضه فقط إذا كانت المهمة خاطئة تمامًا ، أو إذا كان الفريق غير موافق من الناحية المعمارية. وهكذا: نضع موافقة ، ودمج ، وكتابة تعليق - وإذا أراد مؤلف طلب السحب ، فسيعود ويصلحها. هناك ، سوف يصحح الخشونة الصغيرة أو يستخدم نوعًا من الأنماط. ما هي ممارسات الرفض الخاصة بك؟
ألكساندر ماكاروف: المعايير تتوافق تمامًا مع معاييرك - إذا لم تتم المهمة بشكل جيد ، بالطبع ، عليك إنهاءها. حتى لو كان الرمز يبدو جيدًا ومن الناحية المعمارية ، كل شيء رائع.
, : , .
على سبيل المثال ، نقول: "يا رفاق ، لنجري اختبارًا". هناك استثناءات بالطبع ، لكن الاختبارات مهمة جدًا. من الواضح منهم ما إذا كان الشخص قد فهم المهمة بشكل صحيح: مرة أخرى ، إذا كان يختبر الفصول والطرق ، وليس حالة الاستخدام ، فهذا أمر مريب بالفعل. لقد قمنا الآن بإفساد العدوى ، هذا هو اختبار الطفرات. يترك Tulza نفس الاختبارات ، لكنه يعدل الكود ويعمل. وإذا تم تمرير الرمز الذي تم تغييره بنفس الاختبارات ، فلن تكون هناك حاجة لجزء من الكود - يمكنك فقط أن تأخذه ، تحذفه ، ولن يتغير شيء.
قليلا من وراء الكواليس.
أيضًا ، بالطبع ، مشكلات الأمان والأداء - سيكون هناك رفض هنا. نحن لا نرفض تفاهات: إما أن نطلب إصلاحها ، أو نصلحها بأنفسنا - ندفع مباشرة إلى طلبات السحب لمن صنعوها ، ونعرض بالفعل في الكود النهائي ماذا وكيف ولماذا فعلنا ذلك.
انطون موريف:بخصوص ما قلته - هل حلت المشكلة؟ يحدث أن المطور ، أثناء حل مشكلة ما ، قد حل مشكلة أخرى. ليس من الضروري رفض مثل هذه المواقف. أو على الأقل اكتشف كيف تم الإشراف على المهمة.
كونستانتين بوركاليف: لكن لحظة ربط رسائل الالتزام بأداة تعقب المهام تبدو مهمة بالنسبة لي. هناك مهام يومية تساعد كثيرًا فيها. هل تعرف متى: "اسمع ، فعلنا شيئًا مشابهًا في مشكلة الزر". يمكنك العثور على المهمة المتعلقة بالزر ، وفهم الرقم ، والانتقال إلى سجل المستودع ، والعثور على فرق تلك الالتزامات وترى: في الواقع ، لقد أفسدنا هذا وذاك ، يمكن نقله هنا.
لكن العكس يحدث أيضًا. أنا أنظر إلى الكود المصدري لمشروع واحد هنا ووجدت الوظيفة action684 في الخلفية.
أكتب إلى صديق ، أسأل ، ما هذا الاسم الرائع في الكود. يجيب - في إشارة إلى المهمة في المتعقب. "لماذا نبتكر اسمًا لوظيفة ما ، أنت تكتبه كجزء من مهمة" ... Thresh ، التي لا ينبغي أن توجد في عالم عادي ، بالطبع.