Stratégie de mise en place de revues de code

7843bb075c05be6886a97b77e36758ff?s=47 David
April 10, 2015

Stratégie de mise en place de revues de code

GitHub organise des revues de code. Google organise des revues de code. Finalement, la revue de code ne serait-elle pas incontournable pour améliorer la qualité d'un projet ?

Et si on faisait pareil sur notre projet ? Est-ce simple à mettre en oeuvre ? L'équipe jouera t'elle le jeu ? Quels outils utiliser ? Quels gains pour le projet ? Quel intérêt pour l'équipe ?

Cette présentation répondra à ces questions à travers mon retour d'expérience sur un projet nécessitant la mise en place de revues de code.

Devoxx France 2015

7843bb075c05be6886a97b77e36758ff?s=128

David

April 10, 2015
Tweet

Transcript

  1. @dwursteisen #DevoxxFR #CodeReview Stratégie de mise en place de revue

    de code
  2. @dwursteisen #DevoxxFR #CodeReview #DevoxxFR #CodeReview

  3. @dwursteisen #DevoxxFR #CodeReview DEI Formation Consultant

  4. @dwursteisen #DevoxxFR #CodeReview

  5. @dwursteisen #DevoxxFR #CodeReview Cécile Jean-rené Xavier Ezequiel

  6. @dwursteisen #DevoxxFR #CodeReview Cécile Jean-rené Xavier Ezequiel

  7. @dwursteisen #DevoxxFR #CodeReview Moons Air Rail Ferry Car

  8. @dwursteisen #DevoxxFR #CodeReview Moons Air Car Rail Ferry

  9. @dwursteisen #DevoxxFR #CodeReview

  10. @dwursteisen #DevoxxFR #CodeReview Moons Air Car Rail Ferry

  11. @dwursteisen #DevoxxFR #CodeReview Moons Air Car Rail Ferry

  12. @dwursteisen #DevoxxFR #CodeReview Moons Air Car Rail Ferry

  13. @dwursteisen #DevoxxFR #CodeReview Moons Air Car Rail Ferry

  14. @dwursteisen #DevoxxFR #CodeReview Moons Air Car Rail Ferry

  15. @dwursteisen #DevoxxFR #CodeReview Mais qu’est-ce que c’est que CE CODE

    ?
  16. @dwursteisen #DevoxxFR #CodeReview if ("Settings".equals(method)) { Settings request = exchange.getIn().getBody(Settings.class);

    for (Element settingElement : request.elements()) { if (!users.contains(settingElement.getUsername())) { badUsers.add(settingElement.getUsername()); } } }
  17. @dwursteisen #DevoxxFR #CodeReview if ("Settings".equals(method)) { Settings request = exchange.getIn().getBody(Settings.class);

    for (Element settingElement : request.elements()) { if (!users.contains(settingElement.getUsername())) { badUsers.add(settingElement.getUsername()); } } }
  18. @dwursteisen #DevoxxFR #CodeReview if (!"Settings".equals(method)) { return; } Settings request

    = exchange.getIn().getBody(Settings.class); for (Element settingElement : request.elements()) { if (!users.contains(settingElement.getUsername())) { badUsers.add(settingElement.getUsername()); } }
  19. @dwursteisen #DevoxxFR #CodeReview LOG.warn(new StringBuilder("No user found").toString());

  20. @dwursteisen #DevoxxFR #CodeReview LOG.warn(new StringBuilder("No user found").toString()); Fausse optimisation

  21. @dwursteisen #DevoxxFR #CodeReview private static final TO_BE_REPLACED = new char[]

    { '_', ','; '-'}; public static String normalize(final String string) { if (string == null) { return null; } String normalizedString = string; for (char charToReplace : TO_BE_REPLACED) { normalizedString = normalizedString.replace(charToReplace, ' '); } return normalizedString.trim(); }
  22. @dwursteisen #DevoxxFR #CodeReview private static final TO_BE_REPLACED = new char[]

    { '_', ','; '-'}; public static String normalize(final String string) { if (string == null) { return null; } String normalizedString = string; for (char charToReplace : TO_BE_REPLACED) { normalizedString = normalizedString.replace(charToReplace, ' '); } return normalizedString.trim(); } Remplace des caractères spéciaux par des espaces
  23. @dwursteisen #DevoxxFR #CodeReview private static final String PATTERN = "[_,-]";

    public static String trimSpecialCharacters(final String string) { if (string == null) { return null; } return StringUtils.trim(string.replaceAll(PATTERN, " ")); }
  24. @dwursteisen #DevoxxFR #CodeReview private static final String PATTERN = "[_,-]";

    public static String trimSpecialCharacters(final String string) { if (string == null) { return null; } return StringUtils.trim(string.replaceAll(PATTERN, " ")); } Remplace (également) des caractères spéciaux par des espaces
  25. @dwursteisen #DevoxxFR #CodeReview + - ↗ ?

  26. @dwursteisen #DevoxxFR #CodeReview + - ↗ ? Qualité du code

    !
  27. @dwursteisen #DevoxxFR #CodeReview + - ↗ ? Qualité du code

    ! revue de code ?
  28. @dwursteisen #DevoxxFR #CodeReview mise en place de revue de code

    systématique
  29. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265
  30. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265
  31. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265
  32. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265
  33. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265
  34. @dwursteisen #DevoxxFR #CodeReview AUCUNE tâche n’a été passée en REVUE

    DE CODE
  35. @dwursteisen #DevoxxFR #CodeReview + - ↗ ?

  36. @dwursteisen #DevoxxFR #CodeReview + - ↗ ? revue de code

    !
  37. @dwursteisen #DevoxxFR #CodeReview mise en place de revue de code

    quand c’est nécessaire
  38. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265
  39. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265
  40. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265
  41. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265
  42. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265
  43. @dwursteisen #DevoxxFR #CodeReview (toujours) AUCUNE tâche n’a été passée en

    REVUE DE CODE
  44. @dwursteisen #DevoxxFR #CodeReview Code review

  45. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265 Code review
  46. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265 Code review Code review
  47. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265 Code review Code review
  48. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265 Code review Code review
  49. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265 Code review Code review
  50. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265 Code review Code review
  51. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265 Code review Code review
  52. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265 Code review Code review Revue de code nécessaire
  53. @dwursteisen #DevoxxFR #CodeReview A Faire En cours Implémentée Terminée MNS-

    456 MNS-135 MNS- 265 Code review Code review
  54. @dwursteisen #DevoxxFR #CodeReview Comment est faîte une revue de code

    ?
  55. @dwursteisen #DevoxxFR #CodeReview To: jrb@aperture.com Subject : Revue de code

    MNS-435 Salut ! J’ai regardé le code de la tâche MNS-435. Mes retours : Dans la class BookManagerAugmented, je pense que la variable toto, ligne 42 n’est pas très bien nommé. if (toto == null) { return false; } Que penses-tu de changer son nom par tata plutôt ? C’est mieux non ?
  56. @dwursteisen #DevoxxFR #CodeReview To: jrb@aperture.com Subject : Revue de code

    MNS-435 Salut ! J’ai regardé le code de la tâche MNS-435. Mes retours : Dans la class BookManagerAugmented, je pense que la variable toto, ligne 42 n’est pas très bien nommé. if (toto == null) { return false; } Que penses-tu de changer son nom par tata plutôt ? C’est mieux non ? Mauvaise Idée
  57. @dwursteisen #DevoxxFR #CodeReview

  58. @dwursteisen #DevoxxFR #CodeReview Code

  59. @dwursteisen #DevoxxFR #CodeReview Commentaires

  60. @dwursteisen #DevoxxFR #CodeReview if (s == null) { return false;

    }
  61. @dwursteisen #DevoxxFR #CodeReview if (s == null) { return false;

    }
  62. @dwursteisen #DevoxxFR #CodeReview if (s == null) { return false;

    }
  63. @dwursteisen #DevoxxFR #CodeReview if (s != null) { return true;

    }
  64. @dwursteisen #DevoxxFR #CodeReview Quels impacts sur le code ?

  65. @dwursteisen #DevoxxFR #CodeReview if(Integer.ZERO.equals(amount)) { StringBuilder sb = new StringBuilder();

    sb.append("Deal ID is").append(id) .append(" and deal amount is ").append(amount) .append(". "); LOGGER.error(sb.toString()); throw new ValidationException("Price error. Check the price"); }
  66. @dwursteisen #DevoxxFR #CodeReview if(Integer.ZERO.equals(amount)) { StringBuilder sb = new StringBuilder();

    sb.append("Deal ID is").append(id) .append(" and deal amount is ").append(amount) .append(". "); LOGGER.error(sb.toString()); throw new ValidationException("Price error. Check the price"); }
  67. @dwursteisen #DevoxxFR #CodeReview if(Integer.ZERO.equals(amount)) { LOGGER.error("Deal ID is" + id

    + " and deal amount is " + amount + ". "); throw new ValidationException("Price error. Check the price"); }
  68. @dwursteisen #DevoxxFR #CodeReview if(Integer.ZERO.equals(amount)) { LOGGER.error("Deal ID is {} and

    deal amount is {}. ", id, amount); throw new ValidationException("Price error. Check the price"); }
  69. @dwursteisen #DevoxxFR #CodeReview if(Integer.ZERO.equals(amount)) { LOGGER.error("Deal ID is {} and

    deal amount is {}. ", id, amount); throw new ValidationException("Price error. Check the price"); }
  70. @dwursteisen #DevoxxFR #CodeReview if(Integer.ZERO.equals(amount)) { LOGGER.warn("Deal ID is {} and

    deal amount is {}. ", id, amount); throw new ValidationException("Price error. Check the price"); }
  71. @dwursteisen #DevoxxFR #CodeReview Quels impacts sur l’équipe ?

  72. @dwursteisen #DevoxxFR #CodeReview

  73. @dwursteisen #DevoxxFR #CodeReview Binômage

  74. @dwursteisen #DevoxxFR #CodeReview aujourd’hui ?

  75. @dwursteisen #DevoxxFR #CodeReview Il faut porter attention à ce que

    cette pratique ne se dissipe pas
  76. @dwursteisen #DevoxxFR #CodeReview Questions ? (je suis là pour y

    répondre)