כותרות TheMarker >
    cafe is going down
    ';

    פרטי קהילה

    מחקר ופיתוח

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

    אינטרנט והייטק

    פורום

    הנדסת תוכנה

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

    חברים בקהילה (1520)

    אמיר לשם
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין
    משה ,
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין
    bfou
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין
    היזם
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין
    תנועת כמוך
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין
    לואיס קרול
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין
    שחר י
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין
    דורון טל
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין
    רובינזוןקרוזו
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

    המדריך הלא שלם ל-code review

    29/5/09 13:18
    2
    דרג את התוכן:
    2009-06-07 08:03:30
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין


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

     

    אתאר את הגישה שלי ואשמח לתגובות, הערות ותוספות.

     

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

     

    2. יש שתי גישות לביצוע ה-code review. הגישה הראשונה היא גישת ה-offline ובה השינוי המוצע נשלח במייל או מועלה לאתר ייעודי שבו הוא נגיש למפתחים אחרים והם יכולים לכתוב הערות (במייל או באתר עצמו). הייתרון בגישה הזאת הוא בכך שה-review מתבצע בזמן אחר שנוח ל-reviewer ובמקרה של אתר, מאפשר גם שיתוף של כמה אנשים בצורה נוחה יחסית (חבר המליץ לי על תוכנת ה-reviewboard במקרה כזה). אני חושב שה-review יכול להיות אפקטיבי בשיטה כזאת אבל לא בכל המקרים. במקרה שנדרש דיון יסודי יותר ולא רק הערות מסוג "שים לב שהפונקציה יכולה לזרוק exception במקרה כזה וכזה" השיטה העדיפה היא קריאה משותפת של הקוד וסיכום ההערות על נייר או במייל.

    שיטה כזאת של "קריאה משותפת" מתאימה מאוד למפתחים חדשים (בפרוייקט הספציפי ובכלל), אבל גם במקרים אחרים היא הזדמנות לחזור ולבדוק את הדרישות, ה-design המפורט ולקיים על כך דיון קצר. החיסרון בשיטה כזאת הוא חוסר היכולת לשתף כמה reviewers. (אני יודע שיש מקומות שבהם עושים ישיבות review רבות משתתפים וקוראים את הקוד מהמקרן, אבל אני חושב שיש בעיות רבות בשיטה: א' - מוזמנים בד"כ גם אנשים שאין להם הרבה קשר לקוד ויקח רק שעה להסביר להם במה מדובר, ב' - מאותה הסיבה קל לדיון לסטות לכיוונים אחרים ולא קשורים בכלל, ג' - לא כל האנשים אוהבים את המעמד הזה של מעין "משפט שדה" שבו קלונם יכול לצאת ברבים בגלל שגיאה טפשית)

     

    3. מי עושה את ה-review? אם מדובר בשינוי קוד קיים, חייבים לשתף את כותב הקוד המקורי ואם אינו שותף לפרויקט יותר, מישהו שקיבל את האחריות עליו. במקרה של חלק מהמודול - רצוי לשתף את המתכנן של אותו מודול ואת מי שמימש חלקים גדולים ממנו. כך אפשר לעלות על שימוש לא נכון ב-APIs, סטייה מ-invariants או הכנסת state למערכת שצריכה להיות stateless, רחמנא ליצלן. אפשר לעשות את ה-review גם על ידי סמכות מקצועית בקבוצה. אני חושב שלא כדאי לעשות את ה-code review עם מי שיש לו רק הסמכות הניהולית (יכול לקרות בניהול מטריציוני). רצוי גם לדאוג שכל אנשי הפרויקט יתנסו בלעבור code review ובלעשות code review לאחרים. לא צריכים להיות כאן "מיוחסים".

     

    4. טוב, אז מה בודקים?

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

     

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

     

    ג. בדיקות. האם יש לקוד בדיקות (רצוי unit tests) שבודקות את הכתוב?

     

    ד. התאמה לקונבנציות. זה אמנם קשור קצת לקריאות, אבל מקומות שונים מפתחים סט של קונבנציות מקומיות (כמו האם לשים I לפני שם של interface או לא)

     

    ה. טיפול במקרי קצה. עד כמה שנתאמץ לא נוכל לפתח בדיקות שבודקות כל היבט בפעולת המערכת בכל מצב אפשרי. לא את כל ה-exceptions אפשר להכניס לסט הבדיקות, כי למשל לא תמיד אפשר לנתק תקשורת TCP באמצע. אם לא נטפל במקרים אלה עכשיו כנראה שהפעם הבאה אחרי ה-review שנסתכל על הקוד הזה תהיה כאשר הלקוחות יתחילו להתקשר למרכז התמיכה. לכן ה-code review הוא הזמן להיזכר ולפעמים גם להחליט איך אנחנו רוצים שהמערכת תתנהג במצבי שגיאה שלא תוכננו מראש. באותה הזדמנות כדאי לבדוק איך המערכת מתמודדת ומודיעה על שגיאות (למשל עדיף להודיע למתשמש שאין לו הרשאות כתיבה בדיסק מאשר לזרוק exception עם הטקסט "this exception should never be thrown")

     

    ו. נכונות. כן, שמתי את ה-item הזה אחרון, כי מעבר לתכניות פשוטות או שגיאות גסות ובולטות, אני לא חושב שב-review אפשר להתמודד עם בעיות של נכונות. גם בגלל שהזמן שהושקע בפיתוח הושקע בעיקר בנכונות, ובגלל שבדקנו שיש בדיקות (סעיף ג') לעיל, אני חושב שבעיות ב"נכונות" יתגלו במהרה ואם אינן מהותיות הן קלות יחסית לתיקון. אני יודע שיש כאן גם דיעות אחרות.

     

    5. כמה זמן מקצים ל-code review? מהנסיון האישי שלי, למדתי שצריך להקצות זמן ראוי ל-code review, שנע איפשהו באיזור ה-20% מזמן הפיתוח בפועל, והוא כולל גם את הזמן לשינויים. כמובן שזמן זה משתנה מסיטואציה אחת לשניה, אבל חשוב להקצות אותו מראש ולא לאכול את הזמני ה-buffer שהוקצו לפרויקט לצורך ה-review.


    ומכאן, אשמח לשמוע את דעתכם!
    מה אתם חושבים? מעתה קל יותר להוסיף תגובה. עוד...
     

    הוספת תגובה על "המדריך הלא שלם ל-code review"

    נא להתחבר כדי להגיב.

    התחברות או הרשמה   

    29/5/09 18:28
    0
    דרג את התוכן:
    פורסם ב: 2009-05-29 18:28:53
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

    מאוד תלוי באופי החברה וסוג המוצר והפיתוח.

    עבדתי בחברות ללא כל  CODE REVIEW  חברות שעשו את זה רק באופן אישי לפני CHECK IN וחברות שעשו ישיבה רבת משתתפים על כל שורת קוד.

    בהרבה מקרים ה CODE REVIEW היה בזבוז זמן מלבד הואי חברתי, אם אתה אכן מתכוון להשקיע 20 אחוזים מזמן הפיתוח בכך  אולי מוטב להשקיע את הזמן בDESIGN REVIEW .

    CODE REIVEW יכול לתרום כדי לאכוף קונוונציות כתיבה וקריאות.

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

     

    תו רון


    --
    תו-רון (שם עט)

    כותב חובב:
    סיפורים, שירים ותוכנות מחשבים
    30/5/09 11:40
    0
    דרג את התוכן:
    פורסם ב: 2009-05-30 11:40:06
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין


    נכון, אין מתכון אחיד ולכל סיטואציה קיים המתכון המתאים לה בדיוק (למרות שאני חושב שישיבות רבות משתתפים אינן יעילות ברוב המקרים). בד"כ ל-design review מוקדש זמן נפרד, וההבדל הוא שאת ה-design review עושים לפני כתיבת הקוד ואת ה-code review עושים אחר כך. בדרך כלל באותו הזמן נוספות דרישות, מתגלות תובנות חדשות ובין השאר כדאי לדון גם בכך. ה-20% הוא מספר סתמי וכולל בתוכו גם לא מעט תקורות. אפשר להקדיש גם 10%, תלוי במקרה.

     

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


    --
    מנהל קהילת מחקר ופיתוח:
    http://randd.cafe.themarker.com
    30/5/09 21:15
    1
    דרג את התוכן:
    פורסם ב: 2009-05-30 21:15:45
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

    צטט: גנאדי 2009-05-30 11:40:06


    נכון, אין מתכון אחיד ולכל סיטואציה קיים המתכון המתאים לה בדיוק (למרות שאני חושב שישיבות רבות משתתפים אינן יעילות ברוב המקרים). בד"כ ל-design review מוקדש זמן נפרד, וההבדל הוא שאת ה-design review עושים לפני כתיבת הקוד ואת ה-code review עושים אחר כך. בדרך כלל באותו הזמן נוספות דרישות, מתגלות תובנות חדשות ובין השאר כדאי לדון גם בכך. ה-20% הוא מספר סתמי וכולל בתוכו גם לא מעט תקורות. אפשר להקדיש גם 10%, תלוי במקרה.

     

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

     

    כמה חוקים שלי לCODE REVIEW. מצאתי שהם מתאימים לכל סוג חברה, בגדול, ולכל תהליך פיתוחי. הFINE POINTS צריכים קצת להשתנות, אבל בשביל זה יש מנהלים, לא?

     

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

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

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

    3) פגישה אחרת, חד שבועית, של כל הצוות על קוד רנדומלי, לא מוכן מראש, של מישהו. המטרה כאן היא יותר כללית, לראות את הקוד "באמת" נראה, לדון ברמה יותר גבוה על עקרונות תכנון, תכנות, בסיס (כגון JAVA בכלליות) ומבנה. תמיד יש מה ללמוד בפגישות האלה.

     

    אלו שני הסנטים שלי.

     


    --
    -----------
    רמקול - חוות דעת איכותיות על ספקי שירות מקומיים
    http://www.ramkol.co.il
    31/5/09 09:59
    0
    דרג את התוכן:
    פורסם ב: 2009-05-31 09:59:50
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

    צטט: ליאורשיאון 2009-05-30 21:15:45

     

    3) פגישה אחרת, חד שבועית, של כל הצוות על קוד רנדומלי, לא מוכן מראש, של מישהו. המטרה כאן היא יותר כללית, לראות את הקוד "באמת" נראה, לדון ברמה יותר גבוה על עקרונות תכנון, תכנות, בסיס (כגון JAVA בכלליות) ומבנה. תמיד יש מה ללמוד בפגישות האלה.

     

    אלו שני הסנטים שלי.

     

     

    איך אתה  מונע מהפגישות האלה לסטות מהנושא ולהפוך פתאום לדיון design או משהו כזה? איך האנשים מרגישים כאשר לכולם יש מה לומר על הקוד שלהם?


    --
    מנהל קהילת מחקר ופיתוח:
    http://randd.cafe.themarker.com
    3/6/09 23:36
    1
    דרג את התוכן:
    2009-06-06 22:35:06
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

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

     

    א. אני יוצא מהנחה שבארגונים מסודרים עובדים עם מערכת ניהול/בקרת תצורה כזו או אחרת, שאחראית לשקף את סביבות העבודה. ברוב המערכות כיום ניתן לבנות בקלות העתק אמיתי של סביבת העבודה של המבוקר, על מחשב המבקר. "אמיתי" כוונתי לא סתם להעתקה של קבצים, אלא להעתק חכם שלוקח את הגרסאות הנכונות, כולל כל מה שדרוש לכך שסביבת העבודה תרוץ גם מרחוק, וכולל תיעוד של הדברים ששינה המבוקר. בתשובה לשאלה המתבקשת ("למה שלא ישבו שניהם יחד מול מסך אחד?") , אני מסכים שרצוי לעשות את ה- CR בפגישה פיזית, אך לא תמיד זה מתאפשר, בעיקר אם המבקר נמצא פיזית באתר אחר מהמבוקר.

     

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

    1.  להגדיר במכונת המצבים מצב של CR . זהו מצב בפני עצמו, העוזר לדעת עבור כל שינוי , האם עבר CR .

    2. לא לאפשר במכונת המצבים אפשרות שמי שמימש את השינוי, יהיה גם זה שמאשר את ה- CR .

    3.  לא לאפשר להעביר הלאה שינויים, דהיינו לשתף את תוכנם מול שאר סביבות העבודה בפרוייקט,  לפני שה- CR אושר .

    4.  אישור CR יכלול את שם המאשר/מבקר.

     

    ישמו את הכללים הנ"ל, ואיכות הקוד תעלה פלאים. מנסיון אישי.

     

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

     

    תמיר

    GoMidjets - Automate Your Advantage


    נ.ב. הדיון על הנושא יצא מעניין והביא לי רעיון לפוסט לבלוג שלי באנגלית (שמכסה נושאים של ניהול תצורה ומחזור חיי אפליקציה) צוחק
    4/6/09 09:58
    1
    דרג את התוכן:
    פורסם ב: 2009-06-04 09:58:31
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

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

    http://smartbear.com/codecollab-code-review-book.php?stackover 

    5/6/09 23:11
    0
    דרג את התוכן:
    פורסם ב: 2009-06-05 23:11:07
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

    צטט: tgefen 2009-06-04 00:00:19

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

     

    א. אני יוצא מהנחה שבארגונים מסודרים עובדים עם מערכת ניהול/בקרת תצורה כזו או אחרת, שאחראית לשקף את סביבות העבודה. ברוב המערכות כיום ניתן לבנות בקלות העתק אמיתי של סביבת העבודה של המבוקר, על מחשב המבקר. "אמיתי" כוונתי לא סתם להעתקה של קבצים, אלא להעתק חכם שלוקח את הגרסאות הנכונות, כולל כל מה שדרוש לכך שסביבת העבודה תרוץ גם מרחוק, וכולל תיעוד של הדברים ששינה המבוקר. בתשובה לשאלה המתבקשת ("למה שלא ישבו שניהם יחד מול מסך אחד?") , אני מסכים שרצוי לעשות את ה- CR בפגישה פיזית, אך לא תמיד זה מתאפשר, בעיקר עם המבקר נמצא פיזית באתר אחר מהמבוקר.

     

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

    1.  להגדיר במכונת המצבים מצב של CR . זהו מצב בפני עצמו, העוזר לדעת עבור כל שינוי , האם עבר CR .

    2. לא לאפשר במכונת המצבים אפשרות שמי שמימש את השינוי, יהיה גם זה שמאשר את ה- CR .

    3.  לא לאפשר להעביר הלאה שינויים, דהיינו לשתף את תוכנם מול שאר סביבות העבודה בפרוייקט,  לפני שה- CR אושר .

    4.  אישור CR יכלול את שם המאשר/מבקר.

     

    ישמו את הכללים הנ"ל, ואיכות הקוד תעלה פלאים. מנסיון אישי.

     

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

     

    תמיר

    GoMidjets - Automate Your Advantage


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

     

     

    הי תמיר,

     

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

     

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

     

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

     

    גנאדי


    --
    מנהל קהילת מחקר ופיתוח:
    http://randd.cafe.themarker.com
    5/6/09 23:12
    0
    דרג את התוכן:
    פורסם ב: 2009-06-05 23:12:35
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

    צטט: ניר עופרי (nir ofry) 2009-06-04 09:58:31

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

    http://smartbear.com/codecollab-code-review-book.php?stackover 

     

    נראה ספר מעניין, תודה!


    --
    מנהל קהילת מחקר ופיתוח:
    http://randd.cafe.themarker.com
    6/6/09 12:06
    0
    דרג את התוכן:
    פורסם ב: 2009-06-06 12:06:13
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

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

     

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

     

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

     

    לבסוף, כפי שנאמר, יותר חשוב Design Review. 

    6/6/09 22:47
    0
    דרג את התוכן:
    2009-06-06 22:53:24
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

    צטט: גנאדי 2009-06-05 23:11:07

    צטט: tgefen 2009-06-04 00:00:19

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

     

    א. אני יוצא מהנחה שבארגונים מסודרים עובדים עם מערכת ניהול/בקרת תצורה כזו או אחרת, שאחראית לשקף את סביבות העבודה. ברוב המערכות כיום ניתן לבנות בקלות העתק אמיתי של סביבת העבודה של המבוקר, על מחשב המבקר. "אמיתי" כוונתי לא סתם להעתקה של קבצים, אלא להעתק חכם שלוקח את הגרסאות הנכונות, כולל כל מה שדרוש לכך שסביבת העבודה תרוץ גם מרחוק, וכולל תיעוד של הדברים ששינה המבוקר. בתשובה לשאלה המתבקשת ("למה שלא ישבו שניהם יחד מול מסך אחד?") , אני מסכים שרצוי לעשות את ה- CR בפגישה פיזית, אך לא תמיד זה מתאפשר, בעיקר עם המבקר נמצא פיזית באתר אחר מהמבוקר.

     

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

    1.  להגדיר במכונת המצבים מצב של CR . זהו מצב בפני עצמו, העוזר לדעת עבור כל שינוי , האם עבר CR .

    2. לא לאפשר במכונת המצבים אפשרות שמי שמימש את השינוי, יהיה גם זה שמאשר את ה- CR .

    3.  לא לאפשר להעביר הלאה שינויים, דהיינו לשתף את תוכנם מול שאר סביבות העבודה בפרוייקט,  לפני שה- CR אושר .

    4.  אישור CR יכלול את שם המאשר/מבקר.

     

    ישמו את הכללים הנ"ל, ואיכות הקוד תעלה פלאים. מנסיון אישי.

     

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

     

    תמיר

    GoMidjets - Automate Your Advantage


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

     

     

    הי תמיר,

     

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

     

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

     

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

     

    גנאדי

     

     הי גנאדי,

     

    האמת שדיברתי באופן כללי על Code Review וניהול תצורה כקונספט ובאופן כללי, אבל אם כבר הזכרת את ClearCase , שהוא מוצר שלחברה שלי יש מומחיות בפישוט תהליכים שלו ובהקלה על המשתמשים , אני יכול לציין שיש לנו סיפור לקוח (Case Study) עם חברה בינ"ל גדולה שאחד ממרכזי הפיתוח שלה בארץ, ותכננו ומימשנו עבורה תהליך עבודה הכולל התייחסות ל- Code Review , עם ClearCase ו- ClearQuest.

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

     

    תמיר

    GoMidjets

    6/6/09 22:50
    0
    דרג את התוכן:
    פורסם ב: 2009-06-06 22:50:56
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

    צטט: zvikico 2009-06-06 12:06:13


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

     

    לבסוף, כפי שנאמר, יותר חשוב Design Review. 

     

     

    הי צביקי,

    יש לך המלצה על כלי טוב? 

     

    תמיר

    GoMidjets

    7/6/09 07:56
    1
    דרג את התוכן:
    2009-06-07 08:03:30
    1. שלח הודעה
    2. אוף ליין
    3. אוף ליין

    אני מכיר כמה כלים טובים ל-Java ששימשו (או משמשים) אותי אישית:

    •  יש את Checkstyle שהוא כנראה הכי פופולרי היום. 
    • FindBugs מתימרת להיות יותר מעמיקה ומנסה למצוא באגים של ממש בתוך הקוד. 
    • כלי נוסף מעניין הוא Enerjy (כלי מסחרי שהפך חינמי). הוא מופיע כ-Eclipse Plugin בלבד (בניגוד לשני הקודמים שמאפשרים אינטגרציה גם עם סביבות אחרות).


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

     

    אגב, רוב הכלים באים בתצורה אינטרקטיבית (אינטגרציה עם IDE) ובתצורה המאפשרת הפקת דוחות תוך כחלק מביצוע Build. האופציה האחרונה מאוד מועילה למנהלים שבד"כ לא יושבים על IDE אבל אוהבים דוחות. 

     



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

    /null/text_64k_1#

    מה אתם חושבים? מעתה קל יותר להוסיף תגובה. עוד...
     

    הוספת תגובה על "המדריך הלא שלם ל-code review"

    נא להתחבר כדי להגיב.

    התחברות או הרשמה