कैसे कोड समीक्षाएँ तेजी से और अधिक कुशल बनाने के लिए

आमतौर पर कोड की समीक्षा कैसे होती है? आप एक पूल अनुरोध भेजते हैं, प्रतिक्रिया प्राप्त करते हैं, सुधार करते हैं, दूसरी समीक्षा के लिए सुधार भेजते हैं, फिर अनुमोदन प्राप्त करते हैं, और विलय होता है। यह सरल लगता है, लेकिन वास्तव में समीक्षा प्रक्रिया बहुत समय लेने वाली हो सकती है।

कल्पना कीजिए कि आपके पास परिवर्तन की सैकड़ों पंक्तियों के साथ एक पूल अनुरोध है। समीक्षक को कोड को पूरी तरह से पढ़ने और प्रस्तावित परिवर्तनों को समझने के लिए बहुत समय बिताना चाहिए। नतीजतन, एक पूल अनुरोध बनाने से लेकर इसकी मंजूरी तक की पूरी प्रक्रिया में कई दिन लग सकते हैं - यह समीक्षक और परिवर्तनों के लेखक दोनों के लिए बहुत सुखद नहीं है। और मौके अच्छे हैं कि अंत में समीक्षक अभी भी कुछ याद करेंगे। या चेक बहुत अधिक सतही हो सकता है, और सबसे खराब स्थिति में, पूल अनुरोध को तुरंत अस्वीकार कर दिया जा सकता है।

यह पता चला है कि पूल अनुरोध जितना बड़ा होगा, उतना कम लाभ इसे जांचने से होगा।

ऐसी स्थितियों से कैसे बचें? समीक्षक के लिए पूल अनुरोध को आसान और अधिक समझने योग्य कैसे बनाया जाए और पूरी प्रक्रिया को अनुकूलित करें?

हम अपने बैकएंड डेवलपर सर्गेई झूक द्वारा एक लेख का अनुवाद कर रहे हैं कि स्काईेंग मोबाइल ऐप टीम की कोड समीक्षा प्रक्रिया कैसे काम करती है।

श्रेणियाँ बदलें

आइए कल्पना करें कि आपके पास एक कार्य है - परियोजना में नई कार्यक्षमता को लागू करने के लिए। आपके द्वारा काम कर रहे पूल अनुरोध में विभिन्न श्रेणियों के परिवर्तन हो सकते हैं। बेशक इसमें कुछ नया कोड होगा। लेकिन काम के दौरान, आप देख सकते हैं कि कुछ कोड को पहले से रिफैक्ट करने की आवश्यकता है ताकि यह नई कार्यक्षमता को जोड़ने में योगदान दे। या इस नई कार्यक्षमता के साथ, उस कोड में दोहराव दिखाई दिया है जिसे आप समाप्त करना चाहते हैं। या आपको अचानक कोई गलती मिल गई और इसे ठीक करना चाहते हैं। अंतिम पूल अनुरोध कैसा दिखना चाहिए?

सबसे पहले, आइए देखें कि कोड के साथ कौन से परिवर्तन हो सकते हैं।

1. क्रियात्मक परिवर्तन।
2. संरचनात्मक परावर्तन - कक्षाओं में परिवर्तन, इंटरफेस, विधियाँ, कक्षाओं के बीच गति।
3. सरल रीफैक्टरिंग - आईडीई का उपयोग करके किया जा सकता है (उदाहरण के लिए, चर / विधियों / स्थिरांक को निकालना, शर्तों को सरल करना)।
4. नाम बदलने और चलती कक्षाओं - नामस्थान का पुनर्गठन।
5. अप्रयुक्त (मृत) कोड निकाल रहा है।
6. सुधार कोड शैली।

रणनीति की समीक्षा

यह समझना बहुत महत्वपूर्ण है कि इनमें से प्रत्येक श्रेणी की अलग-अलग समीक्षा की जाती है और जब उन पर विचार किया जाता है, तो समीक्षक को विभिन्न चीजों पर ध्यान केंद्रित करना चाहिए। यह कहा जा सकता है कि परिवर्तन की प्रत्येक श्रेणी में अपनी समीक्षा रणनीति शामिल है।

1. कार्यात्मक परिवर्तन: व्यावसायिक समस्याओं और सिस्टम डिज़ाइन को हल करना।
2. स्ट्रक्चरल रिफैक्टरिंग: बैकवर्ड कम्पैटिबिलिटी एंड डिजाइन इम्प्रूवमेंट।
3. आदिम रीफैक्टरिंग: बेहतर पठनीयता। ये परिवर्तन मुख्य रूप से IDE (उदाहरण के लिए, चर / विधियों / स्थिरांक, आदि को निकालने) का उपयोग करके किया जा सकता है।
4. कक्षाओं का नाम बदलना / बढ़ना: क्या नाम स्थान संरचना में सुधार हुआ है?
5. अप्रयुक्त कोड हटाना: पिछड़ा संगत।
6. सुधार कोड शैली: सबसे अधिक बार मर्ज पूल अनुरोध तुरंत होता है।

विभिन्न श्रेणियों में परिवर्तनों की जांच के लिए विभिन्न दृष्टिकोणों का उपयोग किया जाता है, इसलिए, उनकी समीक्षा पर खर्च किए गए समय और प्रयास की मात्रा भी भिन्न होती है।

क्रियात्मक परिवर्तन। यह सबसे लंबी प्रक्रिया है क्योंकि इसमें डोमेन लॉजिक को बदलना शामिल है। समीक्षक यह देखने के लिए देखता है कि क्या समस्या हल हो गई है और जांच करता है कि प्रस्तावित समाधान सबसे उपयुक्त है या इसमें सुधार किया जा सकता है।

स्ट्रक्चरल रिफैक्टरिंग। इस प्रक्रिया को कार्यात्मक परिवर्तनों की तुलना में बहुत कम समय की आवश्यकता होती है। लेकिन इस बारे में सुझाव और असहमति हो सकती है कि कोड को कैसे व्यवस्थित किया जाना चाहिए।

99% मामलों में शेष श्रेणियों की जाँच करते समय, मर्ज तुरंत होता है।

1. सरल रीफैक्टरिंग। क्या कोड अधिक पठनीय हो गया है? - मेरज़िम।
2. कक्षाओं का नाम बदलना / बढ़ना। क्या कक्षा को एक बेहतर नामस्थान पर ले जाया गया है? - मर्ज़।
3. अप्रयुक्त (मृत) कोड हटाना मेरज़िम है।
4. सुधार कोड शैली या स्वरूपण - मर्ज। आपके सहकर्मियों को कोड समीक्षा के दौरान इसकी जांच नहीं करनी चाहिए, यह लिंटर का काम है।

परिवर्तनों को वर्गीकृत क्यों किया जाना चाहिए?

हम पहले ही चर्चा कर चुके हैं कि विभिन्न श्रेणियों के परिवर्तन अलग-अलग संशोधित किए गए हैं। उदाहरण के लिए, हम व्यावसायिक आवश्यकताओं के आधार पर कार्यात्मक परिवर्तनों को सत्यापित करते हैं, और संरचनात्मक पुनर्संरचना में, हम पिछड़े संगतता की जांच करते हैं। और अगर हम कई श्रेणियों को मिलाते हैं, तो समीक्षक के लिए एक ही समय में कई समीक्षा रणनीतियों को ध्यान में रखना मुश्किल होगा। और, सबसे अधिक संभावना है, समीक्षक आवश्यकता से अधिक पूल अनुरोध पर अधिक समय बिताएगा, और इस वजह से, वह कुछ याद कर सकता है। इसके अलावा, यदि पूल अनुरोध में विभिन्न प्रकार के परिवर्तन होते हैं, तो किसी भी सुधार के साथ समीक्षक को इन सभी श्रेणियों को फिर से संशोधित करना होगा। उदाहरण के लिए, आपने संरचनात्मक रिफैक्टिंग और कार्यात्मक परिवर्तन मिश्रित किए। भले ही रीफैक्टरिंग अच्छी तरह से किया जाता है, लेकिन कार्यात्मक के कार्यान्वयन के साथ एक समस्या है, तो सुधारों के बाद समीक्षक को शुरुआत से ही पूरे पूल के अनुरोध को देखना होगा। यही है, रिफैक्टरिंग और कार्यात्मक परिवर्तनों दोनों को फिर से जांचें। इसलिए समीक्षक पूल अनुरोध पर अधिक समय खर्च करता है। रिफैक्टरिंग के साथ एक अलग पूल अनुरोध पर तुरंत विचार करने के बजाय, समीक्षक को एक बार फिर से पूरे कोड की समीक्षा करनी चाहिए।

क्या वास्तव में मिश्रण के लायक नहीं है

एक वर्ग का नाम बदलना / हटाना और उसका पुन: निर्माण। यहाँ हम Git पर आते हैं, जो हमेशा ऐसे परिवर्तनों को सही ढंग से नहीं समझता है। मेरा मतलब है कि बड़े पैमाने पर परिवर्तन जब कई लाइनें बदलती हैं। जब आप एक कक्षा को रिफ्लेक्टर करते हैं और फिर इसे कहीं स्थानांतरित करते हैं, तो गिट इसे आगे बढ़ने के रूप में नहीं समझते हैं। इसके बजाय, Git एक वर्ग को हटाने और दूसरे को बनाने के रूप में इन परिवर्तनों की व्याख्या करता है। इससे कोड की समीक्षा के दौरान प्रश्नों का एक गुच्छा होता है। और कोड के लेखक से पूछा गया है कि उसने यह बदसूरत कोड क्यों लिखा है, हालांकि वास्तव में यह कोड मामूली बदलावों के साथ एक स्थान से दूसरे स्थान पर ले जाया गया था।

किसी भी कार्यात्मक परिवर्तन + किसी भी प्रतिक्रियाशील। हम ऊपर इस मामले पर चर्चा कर चुके हैं। इससे समीक्षक एक बार में दो समीक्षा रणनीतियों को ध्यान में रखता है। भले ही रीफैक्टरिंग अच्छी तरह से की गई हो, फिर भी हम इन परिवर्तनों को विलंबित नहीं कर सकते जब तक कि कार्यात्मक परिवर्तनों को मंजूरी नहीं दी जाती।

किसी भी यांत्रिक परिवर्तन + मनुष्य द्वारा किए गए किसी भी परिवर्तन। "यांत्रिक परिवर्तनों" से मेरा मतलब है कि आईडीई या कोड पीढ़ी का उपयोग करके किया गया कोई भी प्रारूपण। उदाहरण के लिए, हम नई कोड शैली लागू करते हैं और 3000 लाइन परिवर्तन प्राप्त करते हैं। और अगर हम किसी व्यक्ति द्वारा किए गए किसी भी कार्यात्मक या किसी अन्य परिवर्तन के साथ इन परिवर्तनों को मिलाते हैं, तो हम समीक्षक को इन परिवर्तनों और कारणों को मानसिक रूप से वर्गीकृत करने के लिए बाध्य करेंगे: यह एक कंप्यूटर द्वारा किया गया परिवर्तन है - इसे छोड़ दिया जा सकता है, और किसी व्यक्ति द्वारा किए गए इस परिवर्तन की आवश्यकता है जांच करें। इसलिए समीक्षक बहुत से अतिरिक्त समय की जाँच करता है।

उदाहरण

यहां एक विधि फ़ंक्शन के साथ एक पूल अनुरोध है जो क्लाइंट कनेक्शन को धीरे से मेमकेड के लिए बंद कर देता है:



भले ही पूल अनुरोध छोटा है और इसमें कोड की केवल सौ लाइनें हैं, फिर भी यह सत्यापित करना मुश्किल है। जैसा कि आप कमिट्स से देख सकते हैं, इसमें विभिन्न श्रेणियों के परिवर्तन शामिल हैं:

• कार्यात्मक (नया कोड),
• रिफैक्टरिंग (निर्माण / चलती कक्षाएं),
• सुधार कोड शैली (अतिरिक्त डॉक ब्लॉक को हटाना)।

इसी समय, नई कार्यक्षमता केवल 10 लाइनें हैं:



नतीजतन, समीक्षक को पूरे कोड की समीक्षा करनी चाहिए और

• सत्यापित करें कि रिफैक्टरिंग ठीक है।
• जांचें कि क्या नई कार्यक्षमता सही तरीके से लागू की गई है;
• निर्धारित करें कि यह परिवर्तन आईडीई या व्यक्ति द्वारा स्वचालित रूप से उत्पन्न किया गया था।

इसलिए, ऐसे पूल अनुरोध की समीक्षा करना मुश्किल है। स्थिति को ठीक करने के लिए, आप इन कमानों को अलग-अलग शाखाओं में तोड़ सकते हैं और उनमें से प्रत्येक के लिए अनुरोधों का एक पूल बना सकते हैं।

1. Refactoring: वर्ग निष्कर्षण

केवल दो फाइलें हैं। समीक्षक को केवल नए डिजाइन की जांच करनी चाहिए। यदि सब कुछ क्रम में है - मर्जिम।

2. अगला कदम भी रिफैक्टिंग है, हम सिर्फ दो वर्गों को एक नए नामस्थान पर ले जाते हैं

इस तरह के एक पूल अनुरोध की जांच करना काफी सरल है, इसे तुरंत तत्काल किया जा सकता है।

3. अतिरिक्त डॉक ब्लॉक निकालना

यहां कुछ भी दिलचस्प नहीं है। Merzh।

4. कार्यात्मक ही

और अब कार्यात्मक परिवर्तनों के साथ पूल अनुरोध में केवल वांछित कोड शामिल है। इसलिए आपका समीक्षक केवल इस कार्य पर ध्यान केंद्रित कर सकता है। पूल अनुरोध छोटा और जांचना आसान है।

निष्कर्ष

अंगूठे का नियम:

परिवर्तनों की मिश्रित श्रेणियों के साथ विशाल पूल अनुरोध न बनाएं।

पूल अनुरोध जितना बड़ा होता है, समीक्षक के लिए आपके द्वारा प्रस्तावित परिवर्तनों को समझना उतना ही मुश्किल होता है। सबसे अधिक संभावना है, कोड की सैकड़ों लाइनों के साथ एक विशाल पूल अनुरोध अस्वीकार कर दिया जाएगा। इसके बजाय, इसे छोटे तार्किक भागों में तोड़ दें। यदि आपका रीफ़ैक्टरिंग क्रम में है, लेकिन कार्यात्मक परिवर्तनों में त्रुटियां हैं, तो रीफ़ैक्टरिंग को आसानी से वापस आयोजित किया जा सकता है, और इसलिए आप और आपके समीक्षक बहुत शुरुआत से सभी कोड को देखे बिना कार्यक्षमता पर ध्यान केंद्रित कर सकते हैं।

और हमेशा पूल अनुरोध भेजने से पहले इन चरणों का पालन करें:

• पढ़ने के लिए अपना कोड ऑप्टिमाइज़ करें। कोड लिखित की तुलना में बहुत अधिक पठनीय है;
• उनकी समझ के लिए आवश्यक संदर्भ प्रदान करने के लिए प्रस्तावित परिवर्तनों का वर्णन करें;
• पूल अनुरोध बनाने से पहले हमेशा अपना कोड जांचें। और इसे ऐसे करें जैसे यह किसी और का कोड हो। कभी-कभी यह कुछ खोजने में मदद करता है जिसे आपने याद किया। यह आपके पूल अनुरोध की अस्वीकृति की संभावना और सुधार की संख्या को कम करेगा।

मेरे सहयोगी ओलेग एंटोनविच ने मुझे श्रेणियों में परिवर्तन तोड़ने के विचार के बारे में बताया।

संपादक से: सर्गेई प्रोग्रामिंग और पीएचपी के बारे में कई दिलचस्प बातें लिखते हैं, और कभी-कभी हम कुछ का अनुवाद करते हैं: एक स्ट्रीमिंग वीडियो सर्वर , जो HTML फ़ाइलों को प्रदान करता है । इस लेख में टिप्पणियों में उससे सवाल पूछने के लिए स्वतंत्र महसूस करें - वह जवाब देगा!

खैर, हम आपको यह भी याद दिलाते हैं कि हमारे पास डेवलपर्स के लिए हमेशा बहुत सारी दिलचस्प रिक्तियां हैं !