कोड रिव्यू एंटीपैटर्न

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

[एंटीपैटर्न]

The Death of a Thousand Round Trips

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

The Ransom Note

• यह पैच डेवलपर के लिए खास तौर पर महत्वपूर्ण लगता है
• लेकिन रिव्यूअर के लिए यह उतना महत्वपूर्ण नहीं है। इसलिए शक्ति की स्थिति में रिव्यूअर है
• डेवलपर जो बदलाव चाहता है, उसे तब तक बंधक बनाए रखा जा सकता है जब तक रिव्यूअर के लिए महत्वपूर्ण अतिरिक्त काम पूरे न हो जाएँ
  • अतिरिक्त काम का उसी कमिट में शामिल होना ज़रूरी नहीं है, लेकिन वे रिव्यूअर के लिए महत्वपूर्ण काम हैं

The Double Team

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

The Guessing Game

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

The Priority Inversion

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

The Late-Breaking Design Review

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

The Catch-22

• अगर एक बड़ा पैच है, तो वह पढ़ने में बहुत कठिन है। उसे self-contained छोटे हिस्सों में बाँटने की माँग करें
• उल्टा अगर बहुत सारे छोटे पैच हैं, तो उनमें से कुछ अपने-आप में अर्थहीन होंगे। उन्हें फिर से जोड़ने की माँग करें
• अगर किसी खास मामले में किसी तरह का trade-off प्रासंगिक लगता है, तो उसे शिकायत का कारण बनाने के लिए इस्तेमाल करें
  • उदाहरण के लिए, अगर कोड को समझने में आसान बनाया गया है तो वह performant नहीं होगा, और अगर उसे अच्छी तरह optimize किया गया है तो वह पढ़ने और maintain करने में कठिन होगा

The Flip Flop

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

लेकिन गंभीरता से ...

• अभी तक यह लेख एक मज़ाक था। इसका उद्देश्य यह सुझाना नहीं है कि रिव्यूअर जानबूझकर ऐसा बुरा व्यवहार करते हैं
  • ज़्यादातर वर्णन या तो रिव्यूअर के वास्तविक व्यवहार की अतिशयोक्ति हैं, या निराश पैच सबमिटर को जैसा महसूस होता है उसकी अतिशयोक्ति
• असल बात यह है कि ऐसा मत कीजिए!
  • round trip कम से कम रखने की कोशिश करें, छोटी बातों में उलझने से पहले (ज़रूरत हो तो) बड़े rewrite की माँग करें, और पैच की आलोचना करते समय यह भी रचनात्मक रूप से सुझाएँ कि किस तरह का वर्ज़न स्वीकार किया जाएगा
  • अगर पूरे codebase के बारे में आपकी कोई राय है, तो किसी एक डेवलपर के पैच की रिव्यू के दौरान नुक्ताचीनी करने के बजाय सभी डेवलपर्स के साथ व्यापक चर्चा करें
  • अगर आपने गलती से ऐसा किया है, तो जागरूक बनें। मानें कि आपने अनजाने में डेवलपर की ज़िंदगी कठिन बनाई, माफ़ी माँगें, और ज़्यादा मददगार बनने की कोशिश करें
• मूल समस्या शक्ति का दुरुपयोग है
  • जब एक डेवलपर दूसरे डेवलपर के पैच का कोड रिव्यूअर बनता है, तो उसे अस्थायी शक्ति मिलती है। रिव्यूअर के पास उस पैच को commit होने से रोकने की शक्ति होती है
  • शक्ति के साथ ज़िम्मेदारी आती है। शक्ति का इस्तेमाल केवल उसके इच्छित उद्देश्य के लिए और जितना ज़रूरी हो उतना ही करना चाहिए
  • ज़्यादातर एंटीपैटर्न (या जिन हल्के व्यवहारों पर यह व्यंग्य करता है) शक्ति का दुरुपयोग हैं। यानी रिव्यूअर डेवलपर पर मिली अस्थायी शक्ति को लीवर की तरह इस्तेमाल कर पैच सुधार से असंबंधित, या उसके विपरीत, निजी एजेंडा चलाता है
  • यह निजी एजेंडा एंटीपैटर्न के हिसाब से अलग-अलग हो सकता है: असंबंधित काम, निजी स्टाइल पसंद, आलस, बदलाव का विरोध, या पैच सबमिटर के प्रति निजी नापसंदगी
• अगर पैच सबमिटर ने खुद अतीत में कोड रिव्यूअर रहते हुए ऐसा बुरा व्यवहार किया हो, तो नापसंदगी जायज़ भी हो सकती है। इसलिए कोड रिव्यूअर की शक्ति का इस्तेमाल सावधानी से करना चाहिए
  • अगर डेवलपर एक-दूसरे से बदला लेने के दुष्चक्र में फँस जाएँ, तो सॉफ़्टवेयर प्रोजेक्ट बर्बाद हो जाता है

Gatekeeping कोड रिव्यू

• अब तक ध्यान peers के बीच होने वाले कोड रिव्यू पर था
  • कोड रिव्यूअर और पैच सबमिटर सहकर्मी होते हैं; वे एक-दूसरे के मैनेजर नहीं होते और codebase पर अंतिम निर्णय का अधिकार भी उनके पास नहीं होता
  • इसलिए कोड रिव्यू से मिलने वाली शक्ति अस्थायी होती है
• peer review की स्थिति में कोड रिव्यूअर और लेखक के लक्ष्य मूलतः एक जैसे होने चाहिए
  • कौन-सी feature जोड़ी जाए, approval से पहले उसे कितना polish किया जाए, और acceptable coding style क्या है—इन बातों पर अगर गंभीर मतभेद हैं, तो उन्हें कोड रिव्यू के संदर्भ से बाहर सुलझाया जाना चाहिए
• लेकिन दूसरे तरह के कोड रिव्यू परिदृश्यों में ऐसा नहीं होता। खासकर तब बहुत अलग स्थिति होती है जब प्रोजेक्ट के बाहर का कोई व्यक्ति बिना माँगे पैच भेज देता है
  • ऐसा परिदृश्य आम तौर पर free software में होता है
  • क्योंकि दुनिया में कोई भी source code बदल सकता है, और उनमें से कुछ लोग अपने बदलाव वापस भेजने की कोशिश करते हैं
• लेकिन यह अन्य परिस्थितियों में भी हो सकता है
  • proprietary code विकसित करने वाली किसी कंपनी में, एक टीम का डेवलपर दूसरी टीम के प्रोजेक्ट में पैच भेजकर किस्मत आज़मा सकता है
• ऐसी स्थिति में यह संभावना कहीं अधिक होती है कि पैच पाने वाला व्यक्ति शुरू से ही उस बदलाव को नहीं चाहता, या उसके लागू करने के तरीके से पूरी तरह सहमत नहीं है, और इसलिए पैच को बिलकुल स्वीकार न करे
  • इस स्थिति में यह किसी peer reviewer को मिली अस्थायी शक्ति का दुरुपयोग नहीं, बल्कि प्रोजेक्ट के ज़िम्मेदार व्यक्ति के रूप में स्थायी अधिकार का वैध इस्तेमाल है
  • मेरे सॉफ़्टवेयर प्रोजेक्ट की दिशा मैं तय करता हूँ
• जब कोड रिव्यूअर ऐसी 'gatekeeping' भूमिका में हो, तब यह कहकर पैच की आलोचना करना हमेशा गलत नहीं है कि वह मौजूदा सामान्य डिज़ाइन सिद्धांतों या आवश्यकताओं का उल्लंघन करता है
  • हो सकता है कि मुझे यह न पता हो कि उसी समस्या को requirements के अनुरूप कैसे हल किया जाए
  • दरअसल वही कठिन हिस्सा हो सकता है, और शायद यही एकमात्र वजह हो कि मैंने अभी तक वही बदलाव खुद नहीं किया
• लेकिन gatekeeping के संदर्भ में भी बिना स्पष्टीकरण के 'The Guessing Game' लागू करना अशिष्टता है
  • मैं आम तौर पर यह समझाने की कोशिश करता हूँ कि डेवलपर ने कठिन हिस्से को नज़रअंदाज़ किया है, और अगर मुझे खुद जवाब नहीं पता, तो मैं यह भी कहता हूँ
• बेशक 'The Death of a Thousand Round Trips' जैसी निष्क्रिय-आक्रामक रुकावटों के लिए कोई बहाना नहीं है
  • अगर आप सचमुच पैच को कोड में बिल्कुल नहीं लेना चाहते, और gatekeeping भूमिका में आपके पास वह फैसला लेने का वैध अधिकार है, तो आपको यह बात शब्दों में कह देनी चाहिए ताकि सबमिटर अपना समय और बर्बाद न करे

Disclaimer

• मैंने इस लेख के लिए कई वर्षों तक नोट्स इकट्ठे किए हैं—उन कोड रिव्यू से जिनमें मैं शामिल रहा हूँ (दोनों पक्षों से), जो मैंने दूसरों के बीच होते देखे, और जिनके बारे में सिर्फ बातचीत में सुना
• इसलिए यह हाल ही में मेरे कोड की रिव्यू करने वाले किसी खास व्यक्ति को निशाना बनाकर नहीं लिखा गया है

[intajon]

हैरानी की बात है, यह शायद अतिशयोक्ति नहीं भी हो सकती....

[magulhalmom]

यह मैंने सच में झेला है, और लगभग डेवलपमेंट छोड़ने वाला था। सच में फिर से संभलना बहुत मुश्किल था।

[bungker]

इसे पढ़कर लगभग PTSD हो गया था

[kayws426]

लगता है आपने इस लेख के लिए नोट्स अब तक बहुत अच्छी तरह इकट्ठा किए थे!!

[gongsil1212]

सिर्फ पढ़ना भी मानसिक प्रताड़ना जैसा लगता है...

[wedding]

मतलब आख़िरी लाइन ही असली मुद्दा है, है ना? hahaha.....

[jypark]

वाह.. लगा जैसे पागल हो जाऊँगा;;

[ahwjdekf]

ऐसी चीज़ें, si + sm करने वाली जगहों पर जाएँ तो कोरिया में भी आम तौर पर देखी जा सकती हैं। इसे आम बोलचाल में गुटबाज़ी कहते हैं। बुरे लोग अपनी रोज़ी-रोटी बचाने के लिए तरह-तरह की हरकतें करते हैं।

[kenuheo]

काफ़ी शैतानी भरे तरीके हैं।

[gooksangom]

लंबे समय में देखें तो बिना किसी वाजिब कारण के ऐसा व्यवहार करने वाला व्यक्ति आम तौर पर या तो 1) शुरू में ही डेवलपर नेटवर्क से बाहर कर दिया जाता है, या 2) गंदी पर्सनैलिटी के बावजूद इतना सक्षम होता है कि बड़ी ज़िम्मेदारी संभाल रहा होता है और उसे अलग करना मुश्किल होता है, इसलिए कोई न कोई adapter की भूमिका निभाकर communication channel बना रहता है और उसी वजह से उसका connection टिका रहता है; वह बीच का व्यक्ति किसी भी कारण से हट जाए तो ज़्यादा देर नहीं लगती, वह जल्दी ही बाहर कर दिया जाता है। इंसान चाहे जितना भी उछल-कूद कर ले, आखिर काम लोगों को मिलकर ही करना होता है तभी पैसा आता-जाता है, और जब पैसा आता-जाता है तभी लोग भी आते-जाते हैं; इसलिए जो व्यक्ति लोगों के साथ ठीक से नहीं रह पाता, वह अंततः समूह से बाहर कर दिया जाता है और पीछे छूट जाता है.

आमतौर पर किसी समूह में खराब स्वभाव के बावजूद लंबे समय तक टिके रहने वाले लोग अक्सर यह गलतफ़हमी पाल लेते हैं कि वे कोई drama के Sherlock जैसे high-functioning sociopath हैं इसलिए बचे हुए हैं, लेकिन सच यह है कि उनकी उपयोगिता है इसलिए आसपास के लोग बस सहते हुए उनका इस्तेमाल कर रहे होते हैं; जैसे ही उपयोगिता खत्म होती है, रिश्ता "तुम्हारे साथ रहकर गंदा अनुभव हुआ, अब दोबारा मत मिलना" जैसा बन जाता है। Cumberbatch वाला Sherlock भी हमें बाहर से एक आकर्षक sociopath लगता है, लेकिन अगर उसके आसपास ऐसे लोग न होते जो उसे छोड़ते नहीं और उसकी परवाह करते, तो उसकी कोई कहानी ही नहीं बनती.

[dokdok]

खराब इंसानियत के बावजूद जो लोग लंबे समय तक टिके रहते हैं, वे अक्सर यह गलतफहमी पाल लेते हैं कि वे किसी ड्रामा के Sherlock की तरह कोई कमाल के high-functioning sociopath हैं, इसलिए बचे हुए हैं। लेकिन असल में बात सिर्फ इतनी होती है कि उनकी उपयोगिता है, इसलिए आसपास के लोग बस दाँत भींचकर उन्हें झेलते हुए इस्तेमाल करते रहते हैं; और जैसे ही वह उपयोगिता खत्म होती है, रिश्ता इस पर आ जाता है: "तुम्हारे साथ रहकर घिन आई, अब दोबारा कभी मत मिलना।" ==> क्या कमाल की पंक्ति है। इसे याद रख लेना चाहिए।

[savvykang]

यह मुझे workplace harassment या power harassment की याद दिलाता है।

[xguru]

मज़ाक बताया गया है, लेकिन इसे पढ़ते हुए अचानक बहुत चिढ़ होने लगी..