הגדרת סקירת קוד מחייבת לפני מיזוג

מיזוג ללא code review: היכן נשברת הענף הראשי

הענף הראשי הוא המקום היחיד שבו מצב הקוד תואם את מה שיוצא לפרודקשן. כל עוד דחיפה ישירה פתוחה, כל קומיט שעוקף code review מגיע לשם ללא בדיקה: הבנייה נבנית, הבדיקות ירוקות פורמלית, והפגם עולה כבר בתעבורה חיה — בדרך כלל בתרחיש השימוש הגרוע ביותר.

המחיר של שבירה כזו גבוה ממה שנדמה. בנייה שבורה בענף הראשי חוסמת לא את מחבר התיקון: כל הצינור נעצר, משימות של אחרים לא יכולות להתמזג, והשחרור כולו נדחה. חזרה אחורה של מהדורה גוררת איתה שינויים של אחרים — צריך לבחור בין פרודקשן לא יציב לבין אובדן משימות מוכנות שהיו באותה מהדורה.

הגנת ענף הופכת את זה ממוסר אישי לכלל שאי אפשר לעקוף בטעות. הדחיפה הישירה סגורה, מיזוג לענף הראשי אפשרי רק דרך בקשת מיזוג, ונדרשים אישור של סוקר מהתחום והרצה מוצלחת של הבדיקות על סוכן הבנייה. חריגים מתארים בנפרד: תיקון דחוף עובר באותו מסלול, אך עם סט בדיקות מצומצם.

בהמשך מגדירים תור מיזוגים וכללי פתרון קונפליקטים, כדי שמשימות במקביל לא ישברו בנייה זו לזו. אז הענף הראשי מתאים לשחרור בכל רגע, וחזרה אחורה מחזירה סט שינויים אחד מסוים, ולא את עבודת הצוות כולו. נפרק כיצד זה נסגר ברמת הגדרות המאגר והתהליך.

מה נותן code review חובה לפני מיזוג לענף הראשי

כלל הגנת הענף הראשי מוגדר כך שבקשת מיזוג פיזית לא יכולה להתקבל ללא אישור. מספר המאשרים הנדרש, הרצת בנייה ובדיקות חובה, איפוס אישורים עם הופעת קומיטים חדשים — אז האישור מתייחס לרוויזיה שאכן נסקרה, ולא לזו המוקדמת יותר.

ב-GitLab זה מוגדר בהגדרות ענף מוגן וברשימת הבעלים של תחומי הקוד; בפלטפורמות אחרות הלוגיקה משוחזרת באותם כללים.

היכן האפקט נראה במספרים

שיעור הפגמים שנתפסים לפני המיזוג, ולא בפרודקשן, עולה פי כמה. טיפול בערכי קצה, ענפי תנאי שאבדו, עבודה שגויה עם נתונים ריקים והרשאות גישה נראים בסקירת השינויים מיד.

התפוקה תלויה בגודל השינוי: את מאתיים-שלוש מאות השורות הראשונות סוקרים בצפיפות, ובהמשך הקשב יורד — לכן כלל השינויים הקצרים נותן יותר מהוספת סוקרים.

מספר החזרות והתיקונים הדחופים לאחר השחרור מצטמצם: סיבת החזרה כמעט תמיד כבר הייתה קיימת בשינוי, אך נותרה בלי דיון. מהירות הסקירה לעומת זאת עולה — ההערות מגיעות לפני שמעליהן נערמה מנה נוספת של תיקונים.

צפיות של השחרור

השחרור מפסיק להיות תלוי במי שפנוי בערב מסוים. הבדיקות והאישורים מתנהלים במקביל, ותור הבקשות שלא נסגרו נראה במלואו — ברור מה בדיוק מעכב את השחרור ומי צריך לצרף.

אפשרויות בקרת מיזוגים: השוואת גישות

בחירת מצב הבקרה נקבעת לפי מחיר הטעות בענף מסוים ומספר האנשים שכותבים אליו. אין אפשרות אוניברסלית: כללים נוקשים בצוות קטן הופכים להמתנה, וכללים רכים בענף מהדורה מכניסים רגרסיה לפרודקשן. החדשות הטובות — הכללים חיים ברמת המאגר ומשתנים בלי שינויים בקוד.

השילוב עובד בצפיות גבוהה יותר מכל מצב בודד: הגנת הענף הראשי לא מאפשרת לעקוף את התהליך, מספר האישורים קובע את מחיר האישור, ובדיקה סלקטיבית מורידה עומס מהנתיבים המשניים.

בנפרד כדאי לחשוב על איפוס אישורים בקומיטים חדשים ועל מסלולי עקיפה לתיקוני תיעוד — אחרת הצוות יתחיל לחפש פרצות עוקפות כללים.

שלבי הטמעת code review חובה בפיתוח

כללי סקירה נשברים כשמפעילים אותם פורמלית: נדרש אישור, אבל ענפים חיים שבועות, הסוקר מעביר מבט שטחי על השינויים, וחלק מהתיקונים עוברים לענף הראשי ישירות. את הטמעת Code Review חובה מחלקים לשלבים — מהאודיט ועד למסירת התקנון.

1. אודיט של התהליך. בודקים כיצד השינויים מגיעים לענף הראשי כעת: כמה חיים הענפים, מי ומתי סוקר אותם, כמה תיקונים מגיעים אחרי המיזוג. לפי ההיסטוריה מחשבים את שיעור השינויים שלא עברו אף בדיקה.

2. תרשים הסתעפות. בוחרים מודל שמתאים לקצב הצוות: ענפים קצרי-חיים מהענף הראשי, ענפי מהדורה או ענפי סביבות. בנפרד מתארים מסלול לתיקונים דחופים, אחרת יעקפו את הכללים.

3. כללי מיזוג. הגנת הענף הראשי משליחות ישירות, אישור חובה של סוקרים, איפוס אישורים בקומיטים חדשים, בדיקות אוטומטיות — בנייה, בדיקות, ניתוח סטטי — כתנאי למיזוג.

4. פיילוט על צוות אחד. מפעילים את הכללים על שירות אחד ובמשך כמה שבועות בודקים את הזמן מהקומיט הראשון ועד המיזוג, את שיעור ההחזרות, ואת גודל השינויים בבקשה אחת.

5. בדיקה על שינויים אמיתיים. מעבירים דרך הכללים החדשים משימות רגילות ואירועי תקלה. לאירועים משאירים מסלול מואץ: מיזוג בלי המתנה לאישור, אך עם בדיקה חובה בדיעבד.

6. הדרכת הצוות. תקנון, תבנית לתיאור שינויים, צ'ק-ליסט לסוקר, ניתוח הערות אופייניות. את הכללים מטמיעים בהגדרת המוכנות של המשימה, כדי שהסקירה לא תהיה תלויה בזיכרון המשתתפים.

אחרי הפיילוט מעבירים את ההגדרות לשאר המאגרים — ומתקבל סט כללים אחיד במקום סיכומים ידניים.

קונפיגורציות, תקנונים ותיעוד בסיום

בסיום נשארים אצל הלקוח לא גישות למהנדסים שלנו, אלא ארטיפקטים במאגר שלו עצמו: כללי הגנת ענפים, תרשים הסתעפות, מטריצת מאשרים ותקנון שלפיו הצוות עובד בלעדינו.

כל זה נמצא במערכת בקרת הגרסאות לצד הקוד — שינויים עוברים כשינוי רגיל, היסטוריית השינויים נראית, ואפשר להחזיר כלל אחורה באותה דרך כמו קוד.

• קבצי הגדרות הגנת ענפים: איסור דחיפה ישירה לענף הראשי, מיזוג חובה דרך בקשת שינוי, מספר אישורים מינימלי, איפוס אישורים בכל קומיט חדש.
• תרשים הסתעפות: אילו ענפים קיימים, כיצד הם נקראים, כמה הם חיים, מאיזה לאיזה ממזגים ובאילו תנאים — עם דוגמאות על השירותים האמיתיים של הלקוח.
• מטריצת מאשרים: מי מאשר ואילו נתיבים — בעל השירות, מהנדס התשתית, האחראי על האבטחה. מיושם כקובץ אחראים לפי נתיבים (CODEOWNERS) או מקבילה בפלטפורמת האחסון שלכם.
• תבנית לתיאור שינוי: מה משנים, לשם מה, כיצד נבדק, תוכנית חזרה אחורה, קישורים למשימות. תבנית ריקה לא עוברת בדיקה בצד המיזוג.
• סט בדיקות אוטומטיות המקושרות למיזוג: בנייה, בדיקות, ניתוח סטטי, איתור סודות — עם ציון אילו מהן חוסמות ואילו אינפורמטיביות.
• תקנון לעובדים חדשים: מה לקרוא ביום הראשון, כיצד מקבלים הרשאות, מי מאשר את השינוי הראשון, לאן פונים במצב שנוי במחלוקת סביב אישור.
• הוראות תחזוקה: כיצד מוסיפים מאשר חדש, כיצד מרפים כלל לתיקון דחוף וכיצד מחזירים לאחר מכן את המצב המחמיר.

מקרה בוחן: פחות חזרות אחורה וסקירת שינויים מהירה יותר

צוות של ארבעה-עשר מהנדסים בארבעה אזורי זמן תחזק שירות עם מהדורות כמה פעמים בשבוע. השינויים עברו לענף הראשי ישירות, בלי בדיקה: המחבר חשב שהתיקון טריוויאלי, ועמית באזור זמן אחר נודע לו על כך רק אחרי הפריסה.

כל מהדורה חמישית הסתיימה בחזרה אחורה, וסקירת שינוי אחד ארכה בממוצע כארבעים דקות — לפי יומנים והתכתבויות שחזרו מי ולשם מה נגע במודול.

מה שינו: ב-GitLab הפעילו הגנת ענף ראשי ואישור חובה לפני מיזוג. למודולים קריטיים נדרשים שני אישורים, ולשאר מספיק אחד — אחרת הכלל מתחיל לעכב תיקונים קטנים.

לתבנית התיאור הוסיפו סעיפים "כיצד לבדוק" ו"כיצד לחזור אחורה", ואת גודל השינוי הגבילו: תיקון גדול מפוצל לחלקים שאפשר realmente לבדוק בגישה אחת.

מדדי התהליך נלקחו אחרי רבעון. שיעור החזרות ירד מחמישית למקרים בודדים למאה מיזוגים, וזמן סקירת השינוי הצטמצם מארבעים דקות לעשר-שתים עשרה: ההקשר של התיקון והדיון בו נמצאים במקום אחד.

שיעור המשימות שחזרו לעבודה נוספת, לעומת זאת, עלה — וזה צפוי, כי פגמים החלו להימצא בשלב הבדיקה, שבו תיקון אורך דקות, ולא בפרודקשן, שבו הוא הופך לבירור אירוע בלילה.

לצוות מבוזר חשוב משהו אחר: code review חובה העביר את הדיון להערות על השינוי, והוא נשאר בהיסטוריה לצד הקוד. הידע על המודולים הפסיק להיות תלוי באדם אחד, ומהנדס חדש נכנס לפרויקט דרך סקירות של תיקונים אמיתיים, ולא דרך הסברים בעל פה. התקנון חי בהגדרות המאגר, ולא בסיכומים אישיים — ולכן אינו נשחק עם גדילת הצוות.

מה לעשות אם code review הוטמע אבל לא עובד

הכלל קיים, אבל המיזוג עובר בלי אישור — כלומר הגדרת הענף המוגן לא תואמת את התהליך. סיבה אופיינית: מספר האישורים המינימלי הוגדר, אבל איפוס אישורים ישנים לא הופעל, והמחבר מוסיף קומיט אחרי הסקירה, בעוד האישור הקודם ממשיך להיספר. פגם שני — בדיקות סטטוס חובה מפספסות בנייה אם היא לא הסתיימה, ולא אם היא נפלה.

עקיפה אדמיניסטרטיבית היא סיבה נפרדת, והיא ארגונית ולא טכנית. אם להרשאה לעקוף את ההגנה זכאים חצי מהצוות, כל תיקון דחוף עובר בלי בדיקה, והכלל מפסיק להיות כלל. זה לא מתוקן בשכנוע: הזכות לעקוף נשארת אצל מהנדס אחד-שניים, כל מיזוג כזה נרשם ביומן ונסקר ברטרוספקטיבה. כשהעקיפה גלויה, מפסיקים להשתמש בה כמסלול רגיל.

חריגים תפעוליים נחוצים, אבל צרים ומתוארים מראש: ענפי מהדורות, מיגרציות סכמה, קונפיגורציות פריסה. לנתיבים קריטיים ממנים בעלי קוד — אז האישור ניתן על ידי מי שאחראי על המודול, ולא על ידי העמית הראשון שהתפנה, והאישור הפורמלי מפסיק לעבור.

בנייה מחדש של התהליך נעשית בלי לעצור את הפיתוח: מפעילים את הכללים בתורים. תחילה נתיבים קריטיים במצב אזהרה, אחר כך בודקים מדדים — שיעור המיזוגים בלי אישור, הזמן עד להערה הראשונה, גודל הדיף — ורק לאחר מכן מפעילים חסימה. דיפים של מאות שורות לא קורא אף אחד, ולכן הגבלת גודל השינוי נותנת יותר מעוד מאשר חובה אחד.

משמעות הבנייה מחדש היא להחזיר לבדיקה את תפקידה: הכלל צריך לתפוס פגם, ולא ליצור ריטואל. אם אחרי הפעלת החסימות עולה שיעור התיקונים לפי הערות, וזמן האספקה לא יורד, סימן שההגדרה נשארה פורמלית.

ספקות טכניים נפוצים לפני הטמעה

ההתנגדויות לפני הטמעת code review חובה בדרך כלל נשענות לא על הרעיון, אלא על הפחד ש"הזרימה תיעצר". נפרק ספקות אופייניים ברמת המהנדס — ולכל אחד יש מענה ברמת התהליך, ולא הבטחה ש"יהיה טוב".

שינויים גדולים יצטרכו פיצול?

כן, וזה בדיוק הרווח העיקרי. השינוי מתפצל לפי שכבות: בנפרד מודל הנתונים, בנפרד הלוגיקה, בנפרד הממשק. הסוקר קורא בלוקים של כמה מאות שורות ומחזיק את ההקשר, ושיעור הפגמים שהוחמצו יורד פשוט מפני ששום דבר לא נסרק "באלכסון".

מה עושים עם תיקונים דחופים?

להם מחזיקים מסלול נפרד: ענף אחד, אישור של מהנדס תורן, והרצה אוטומטית חובה. המהירות נשמרת, אבל התיקון לא חומק מההיסטוריה — אחרי שחזור השירות עדיין עוברת עליו סקירה.

לשם מה סקירה ידנית אם יש בדיקות אוטומטיות?

האוטומציה תופסת תחביר, רגרסיה והפרות של כללי עיצוב, אבל לא רואה שגיאות במודל הנתונים, פניות מיותרות לשירותים חיצוניים והחלטות שבעוד חצי שנה יהפכו לצוואר בקבוק. הבדיקות מורידות רעש, והאדם מסתכל על הלוגיקה.

נדון במשימה: כיצד להגדיר סקירה בצוות שלכם

הגדרת code review חובה היא לא סימון וי בהגנת הענף, אלא הסכמה על תהליך שצריך לשרוד עומס ותיקון דחוף.

הכלל "צריך לפחות מאשר אחד" כשלעצמו כמעט לא מבטיח דבר: בלי בעלי ספריות, תיקונים במודול התשלומים יאושרו על ידי אדם אקראי, ובלי הגבלה על גיל הענף התור למיזוג יגדל מהר יותר ממה שהצוות מספיק לקרוא קוד.

מה נבהיר בשיחה לפני שמפעילים משהו:

• תרשים הסתעפות: ענפי מהדורה במקביל או ענפים קצרי-חיים מהענף הראשי — מזה נגזר סט כללי ההגנה לכל אחד.
• הרכב הצוותים והתפקידים: כמה אישורים לדרוש, מי מחזיק בספריות דרך קובץ CODEOWNERS, כיצד מפזרים את העומס כדי שהסקירה לא תיתקע על אדם אחד.
• אזורי קוד קריטיים: אימות, תשלומים, מיגרציות נתונים — שם נקבע סף אישורים נפרד ובעל חובה.
• סדר המהדורות: כיצד הסקירה משתלבת בהקפאה, בתיקונים דחופים ובחזרה למהדורה קודמת.
• תוכנית הטמעה: תחילה מצב אזהרה בלי חסימת מיזוג, אחר כך חובה — האספקה לא נעצרת ביום הראשון.
• הרשאות וחריגים: מי יכול לעקוף את החסימה וכיצד כל פעולה כזו נשארת ביומן.

כדי לדון במשימה בצורה עניינית, תארו את גודל הצוות, את תרשים ההסתעפות ואת מה שכבר מוגדר. נחזור עם הערכה ותוכנית הטמעה לפי שלבים.