אז מה היה לנו?

17 באוקטובר 2010

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

 

אז מי עושה Code Review?
השאלה הראשונה שאתם צריכים לשאול את עצמכם היא מי הולך לעשות את ה – Code Review. התשובה הנפוצה היא כמובן ראש הצוות! זה פתרון מצויין כמובן, מכיוון שעל פי רב הוא זה שמכיר את ההיסטוריה של המערכת, יש לו נסיון עם הקונבנציות שנבחרו והוא ער לחריגות. החסרון בשיטה הזאת שהיא מייצרת איים של מידע בתוך גוף הפיתוח. אם כל ראש צוות עושה לאנשים שלו, כנראה שהתוצאות יהיו שונות בכל אחד מהצוותים.
עם בעייה זו ניתן להתמודד בשתי צורות. הראשונה היא ע"י ביצוע Code Review מדגמי ע"י רמה גבוהה יותר (לדוגמה ראש קבוצה שיתלווה ליום של Code Review בכל צוות אחת לשבועיים). החלופה השנייה היא שימוש תדיר בשיטת ה – Peer Review. שיטה זו מאפשרת ביצוע מעבר על הקוד בין מהנדסים מצוותים שונים. היתרון הגדול ביותר של שיטה זו, היא היכולת להקטין בעיות בתהליכי אינטגרציה, כאשר שני מתכנתים מצוותים שונים שעובדים על שני צידיו השונים של הממשק מבצעים את ה – Code Review אחד של השני. כמובן שחוסר הניסיון של חלק מהמהנדסים יכול להכביד משמעותית על התהליך.

 

כולם יודעים לעשות Code Review?
אנחנו בחרנו בשימוש שיטת ה – Peer Review שמאפשרת עבודה במבנה ארגוני שטוח ופתוח יותר. על מנת להפוך את השיטה ליעילה הזמנתי את אורי לביא, סמנכ"ל הפיתוח של PicScout, ומייסד קבוצת ה – Software Craftmanship in Israel להרצות לאנשי הפיתוח על Code Review, איך עושים את זה נכון? ממה צריך להמנע? ולמה צריך לשים לב?
ההרצאה נערכה במשרדי החברה כחלק מתוכנית החלפת ההרצאות ILTechTalk@, שאליה אתם יכולים להרשם, להזמין הרצאות שאתם מעוניינים בהן, לבוא להקשיב להרצאה או להציע הרצאה בתחום המומחיות שלכם, והכל ללא תשלום (אם אתם מתעקשים, אתם ממש לא חייבים להרצות כדי לקבל הרצאה אצלכם במשרד).
ההרצאה התמקדה בפיתוח מונחה עצמים, אך כמובן שכל אחד מכם יכול להתאים את תוכן הטיפים לצרכיו.

 

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

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





  1. מה היעד? על הנווט לתאר באופן ממצה את הבעיה שניסה לפתור.


  2. מה הדרך? על הנווט להציג בראיית על את המערכת (Top-Down) כולל האסטרטגיה לפיתרון, ארכיטקטורת המערכת, ה – Unit Tests,  ובמקרה הצורך להציג את ה – Class Diagram. ולא, לא צריך לעשות את זה ידנית. כיום רב כלי ה – IDE הנפוצים מאפשרים לעשות את זה אוטומטית.


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

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


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




  1. כפילויות קוד: כמה פעמים גיליתם שכמויות קוד מסחריות אצלכם במוצר משוכפלות בסגנון עדות ה – Copy-Paste?  כדי להמנע מהמצב הזה יש כלים כדוגמת CloneAnalyzer לסביבת Eclipse. כלים אלו מאפשרים במבט מהיר לראות באילו ספריות ומודולים יש הכפלת שורות ולטפל בהם. התוצאה פחות שורות קוד במערכת, תחזוקה קלה יותר ופחות באגים בעתיד.


  2. רמת סיבוכיות: כמה If/While וקינונים יש? כמה Unit Tests בודקים את זה? האם יש תאימות? אם אין תאימות, יש סיכוי סביר שאתם עם קוד בעייתי. גם לבדיקת תופעת ה – Cyclomatic Complexity יש כלים שמאפשרים זיהוי מה רמת איכות הקוד כדגומת NDepend.


  3. כיסוי קוד: האם הקוד שאתם כותבים באמת נבדק ע"י ה – Unit Tests שגם אותם כתבתם. את התשובה לכך ניתן למצוא ע"י כלים לניתוח Code Coverage כדוגמת (TestDriven.Net (http://testdriven.net/. כלים אלו בודקים את אחוז הקוד שכוסה ע"י הרצת הבדיקות ומציגים את הפערים.

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




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


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


  3. לא מתקנים תוך כדי טיסה: זיהתם בעייה? על הנווט לתעד את ההערות בצד (ולא על המחשב שבו אתם עושים את סקירת הקוד) ולתקן אותם לאחר הנחיתה.

טיפים של אלופים
לא יודעים בדיוק מה לחפש? בשביל זה המציאו את המונח קוד בריח של פעם (באנגלית Code Smells) אוסף של מספר עשרות סימנים לקוד שלא הייתם רוצים לגעת בו במקל (אבל כמו כל דבר בחיים, לא ממש מומלץ להיות פאנטיים).




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


  2. שימוש ב – Ref וב – Out: אם אתם צריכים להעביר יותר ממשתנה אחד בחזרה, השתמשו ב – Class Members או לחילופין החזירו משתנה אחד שיכלול את כל המשתנים. אם אין קשר בין המשתנים, כנראה שהפונקציה עושה יותר מדבר אחד, ולכן מפספסת במקצת את התכנון.


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


  4. שימוש ב – Switch: השתמשו בפולימורפיזם כתחליף.


  5. עבודה שלא לפי ה – Newspaper Paradigm: רוצים לדעת יותר על פרדיגמה זו? דלגו מספר שורות מטה…



החיים זה כמו עיתון
בכל עיתון יש כותרת ראשית, כותרות משנה ולאחר מכן המשך הכתבה. קוד מתנהג בדיוק אותו דבר, אם יש לנו פונקציה ארוכה מדי, אף אחד לא יקרא אותה. לכן אנחנו רוצים להגיע למצב שבו יהיו לנו כותרת מתומצתת (שם הפונקציה הראשית), כותרות משנה (שמות של פונקציות משנה) ותוכן שיתפצל בין פונקציות המשנה. זוהי ה – Newspaper Paradigm שצריכה להנחות אותנו בעת הפיתוח ובעת ביצוע ה – Code Review. פרדיגמה זו מסייעת לנו עם עוד מספר סימנים:




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


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


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


  4. אורך מחלקות: כאשר Class גדול מ 50-80 שורות (וכולל מעל 2-3 שדות, ו -5-8 מתודות), כנראה שגם כאן אנחנו בבעיה.

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




  1. המנעו מתכנון למשהו שאף פעם לא יקרה… (או KISS): קרה לכם פעם שהתכוננתם למשהו שלא יקרה? עשיתם פיל מבקשה שהגיעה מהשיווק כי חשבתם שזה שרצוי לתמוך בעוד עשר אפשרויות לכל מקרה שלא יקרה? מומלץ להמנע מבניית פילים לפני שמגיעה בקשה חוזרת שהמשמעות שלה שבאמת צריך פתרון בגודל של מפלצת.


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


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


  4. לא לרוץ לפתור בעיות ביצועים: יש לנו נטייה לרוץ ולהכריז שכל דבר שזז הוא בעיית ביצועים. לפעמים יש דגל שחור מעל אזורים מסויימים (לדוגמה אל תעשו שאילתות על טבלאות לוג שמתנפחות במליוני רשומות ביום), אבל ברב המקרים אנחנו נשקיע מאמצים גדולים בקטעי קוד שבפועל אין בהם בעיות ביצועים. את בעיות הביצועים מומלץ לבדוק באמצעות פרופיילר ובדיקות עומסים בנקודות המסירה של המוצר (ואם אתם עובדים ב – SCRUM תתקלו בבעיות בשלבים מוקדמים יחסית, ולא תתנפצו עם הבעיות הביצועים דקה לפני שאתם מגיעים ללקוח).

 קבוצות מבוזרות
אם אתם עובדים בקבוצות מבוזרות על פני הגלובוס (Off-Shore/Near-Shore), יתכן מאוד שאתם צריכים כלי שיעזר לכם. Review Board בעל הקוד החופשי יכול לעזור לכם במשימה. מדובר על כלי מבוסס Web שמאפשר לכם לשלוח ולקבל בקשות ל – Code Review, לבצע אותן ולרשום הערות, ללא צורך בנוכחות פיזית. הכלי תומך בסביבות ניהול הקוד SVN  ו – Git. בסרטון שלמטה תוכלו ללמוד קצת על הכלי ועל היכולות שלו שכוללות השוואות בין גרסאות:


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


פספסתי משהו? מרגישים שיש לכם שיטה מנצחת? רוצים לשתף אותנו בהצלחות שלכם? הרגישו חופשי להוסיף הערה ולתרום לכולנו

 

נהנית מהפוסט? רישום לעידכוני הדוא"ל של הבלוג הפתוח למנהל הפיתוח יבטיח לכם עדכונים חדשים ישירות לדוא"ל!

 

ממשיכים לפתח,
משה קפלן

 

נ"ב ביום שני (היום) יערך מפגש נוסף של ה – Software Craftmanship in Israel. אתם מוזמנים.

 

עדכונים ותגובות של קוראים


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



  1. במקרה של קטע קוד גדול אני ממליצה על מעבר ראשון ב – Off Line כדי ללמוד את התוכן בשקט. לאחר מכן ניתן לבצע את ה – Code Review המשותף בזמן קצר יותר ויעיל יותר. השיטה הזאת יעילה גם במקרים של עבודה עם קבוצות פיתוח ב – Off Shore.


  2. הדוגמאות שהוצגו פה מתמקדות בעיקר בתכנות מוכוון עצמים. בתכנות Embedded שמיועד ל – Real Time יש עלות גבוהה לקריאות מרובות לפונקציות, ולכן צריך לעשות את כל התהליך בזהירות רבה יותר. לעיתים הפונקציות ארוכות יותר, ולכן חשוב לשים דגש על התיעוד.


  3. אל תשכחו לבצע Code Review חוזר במקרה שעלו נושאים מהותיים לטיפול במהלך ה – Code Review.

 

הטיפים של רן תבורי (מתוך ההרצאה באומני התוכנה, המצגת המלאה זמינה ב – http://bit.ly/scilcodereview):

מי יכול לעשות Code Review:




  1. ראש הקבוצה/מנהל הפיתוח.


  2. ראש הצוות.


  3. הארכיטקט.


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

 רן ממליץ על מעבר על הקוד קודם לכן בלי העמית, אבל הכל נתון לבחירתכם.


איך שומרים על ה – Code Review אפקטיבי:




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


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


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


  4. כללי אצבע: פחות משעה לסיבוב ולכל היותר 200 שורות.

הוסף תגובה
facebook linkedin twitter email

כתיבת תגובה

האימייל לא יוצג באתר. שדות החובה מסומנים *

21 תגובות

  1. רן בר-זיק18 באוקטובר 2010 ב 8:32

    פוסט נפלא וחשוב מאד – Code Review לא רק תורם למוצר איכותי יותר אלא גם הופך מתכנתים לטובים יותר.

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

    הגב
  2. Moshe Kaplan18 באוקטובר 2010 ב 8:50

    הי רן,

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

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

    ממשיכים לפתח,
    משה קפלן

    הגב
  3. Uri Lavi18 באוקטובר 2010 ב 9:33

    משה,
    תודה על הפוסט המדהים!
    הערה אחת בהמשך ליוספה.
    הכללים שלי חלים בד"כ על מערכות OO שאינן משובצות. במערכות משובצות ישנן כללים אחרים שצריך לקחת בחשבון.
    ישנו סט של code smells שרלוונטי יותר למערכות אלו (משובצות), אבל זה כבר נושא למפגש (פוסט) אחר 🙂

    הגב
  4. Moshe Kaplan18 באוקטובר 2010 ב 10:11

    הי אורי,

    קודם כל תודה (שוב) על ההרצאה המצוינת,

    אני אדבר עם יוספה, ונראה מה אפשר לעשות בנושא 🙂

    ממשיכים לפתח,
    משה קפלן

    הגב
  5. עודד19 באוקטובר 2010 ב 9:35

    פוסט תמציתי ומעולה.

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

    אגב ב-Visual studio קיים Code analytic שניתן להתאים אותו ע"פ הסטנדרטים של הפרויקט וכך לחסוך זמן בדיקות של Name Convention, הוספת Remarks וכ"ו. 

    הגב
  6. Moshe Kaplan19 באוקטובר 2010 ב 9:54

    שלום עודד,

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

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

    לגבי קונבנבציות, אני מסכים בחהלט, ולאנשי ה – .Net אני רק יכול להמליץ על Resharper שיוכל לעזור לכם בנושא,

    ממשיכים לפתח,
    משה קפלן

    הגב
  7. גרי רשף20 באוקטובר 2010 ב 7:56

    בתור מפתח ששונא Unit Test ו-Code Review – נהנתי לקרוא, וסיכמתי לעצמי באנחה שכנראה אין ברירה וחייבים את זה..

    הגב
  8. Moshe Kaplan20 באוקטובר 2010 ב 9:29

    שלום גרי,

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

    ממשיכים לפתח,
    משה קפלן

    הגב
  9. Korma30 באוקטובר 2010 ב 19:59

    למפתחים ב-JAVA ומשתמשים ב-Maven יש פלאגין בשם Sonar אשר סוכם את אשר מפורט כאן ומעבר: PMD, CHECKSTYLE, CPD, FINDBUG, COBERTURA ו-JUNIT

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

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

    חמור מכך, בעולם ה-Agile, בו אין היררכיה בין אנשי צוות, מגיע מצב שבו כמוביל טכני של צוות אתה מבקש מתכניתן שהחליט שבמקום if-else לגיטימי להשתמש ב-try-catch. ולאחר הבקשה לתיקון הקוד – הוא פשוט מתעלם…

    לא ייאומן כי יתועד ב-revision history של הקובץ האומלל ההוא.

    הגב
  10. תום30 באוקטובר 2010 ב 22:53

    גם אם הקוד יעמוד בכל המדדים שציינת עדיין

    אין שום הבטחה שהוא יהיה קוד טוב.

    זה CODE REVIEW ברמה מאוד שטחית לפי דעתי.

    הגב
  11. Moshe Kaplan30 באוקטובר 2010 ב 23:10

    שלום תום,

    לצערי אף אחד לא נותן לך הבטחה לשום דבר (גם אם תקנה פוליסה בחברת ביטוח יש סיכוי שלא תקבל יותר מדי בסופו של דבר).
    השאלה היא אם ה – Code Review ישיג את המטרה שלו, והאם בחוק ה – 80/20, נשיג את ה – 20% שיתנו את ה – 80% ערך.

    אני אשמח לשמוע על ההמלצות שלך בנושא, ואיך להשיג יותר,

    ממשיכים לפתח,
    משה קפלן

    הגב
  12. Moshe Kaplan30 באוקטובר 2010 ב 23:17

    שלום Korma,

    טיפים מצויין,

    אם כל שורת קוד נכנסת לאחר Code Review אפשר לשלב את הבדיקה לפני ההכנסה.
    דבר נוסף שניתן לבצע, הוא לשלב את הבדיקות האלו בשלב ה – Build במימוש של CI. כך שאם מתכנת "זרק" על הבדיקות ה -Build יכשל.

    הבחירה ב – Agile היא לא בחירה ב"לזרוק" אלא תרבות ארגונית שצריך להשתלב בה.

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

    ממשיכים לפתח,
    משה קפלן

    הגב
  13. Korma31 באוקטובר 2010 ב 2:05

    שלום משה,

    ניתן ואף הצעתי למנהל הפיתוח אצלנו לשלב טריגר ב-Perforce (כלי ניהול תצורת הקוד שעמו אנחנו עובדים) לפיו קוד אשר לא עומד בקריטריון של PMD לפי כללים שהסכמנו עליהם – יידחה.
    כנ"ל לגבי הקריטריונים לקביעת Build מוצלח.

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

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

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

    שבוע טוב ואחלה בלוג.

    הגב
  14. Moshe Kaplan31 באוקטובר 2010 ב 9:28

    שלום Korma,

    1. האם בררת עם מנהל הפיתוח למה יש לחץ על קצב העבודה? לעיתים יש נקודות בפיתוח שאתה צריך להקריב משהו בטווח הקצר כדי לזכות בהישג גדול יותר. (אנחנו מבצעים מיזוג של שני מוצרים בימים אלו, ונאלצים בגלל זה להקטין את ההשקעה באוטומציה, מתוך הנחה שלאחר המיזוג ניתן יהיה לבצע אותה בחצי מאמץ מהנוכחי).
    2. אם הסיבה היא לא קצרת טווח, או אין סיבה אמיתית שאתה מתחבר אליה, או שזה גורם לך רמות תסכול שגורמות לך לא להינות מהקימה בבוקר לעבודה, אני בטוח שיהיו מספיק מקומות מעניינים שישמחו לקלוט אותך.

    ממשיכים לפתח,
    משה קפלן

    הגב
  15. Korma1 בנובמבר 2010 ב 9:32

    שלום משה,

    היות ואני מה שמכונה אצלנו "עוגן טכנולוגי" וגם זה שדאג ליישם את ה-CI ברובו עבור המוצר שלנו – אני מרגיש את תחושת התסכול העולה מהכתוב.

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

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

    🙂

    הגב
  16. Moshe Kaplan1 בנובמבר 2010 ב 9:53

    שלום Korma,

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

    ממשיכים לפתח,
    משה קפלן

    הגב
  17. Rotem Bloom5 ביולי 2011 ב 17:51

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

    תודה על השיתוף והפירוט המעולההההההה

    הגב
  18. Moshe Kaplan5 ביולי 2011 ב 20:45

    תודה רותם ובהצלחה בהטמעה!

    ממשיכים לפתח,
    משה קפלן

    הגב
  19. עפר6 ביולי 2011 ב 14:38

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

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

    לא אמליץ כאן על כלי מסויים מטעמים מובנים.

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

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

    הגב
  20. שמאי1 באוגוסט 2011 ב 13:17

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

    שנית, אולי מישהו מכיר מוצר טוב שעושה Code Review יעיל ומועיל לסביבת VSTS 2010 בC# כשפלפורמת הטרגט היא Web-ית פנימית וחיצונית ?

    הגב
  21. Moshe Kaplan1 באוגוסט 2011 ב 13:41

    שלום שמאי,

    קודם כל תודה!
    ל – Review Board יש תוכניות עתידיות לבצע את זה, אני מאמין שבהשקעה לא גדולה אפשר לבצע את זה:
    http://code.google.com/p/reviewboard/wiki/Summer_of_Code_Ideas

    מלבד זאת אתה מוזמן להעיף מבט בשיחה הבאה שמפרטת את כל הכלים הזמינים בשוק:
    http://stackoverflow.com/questions/5369683/code-review-plugin-for-visual-studio-and-tfs

    ממשיכים לפתח,
    משה קפלן

    הגב