Words on code reviews 🢅5

La idea no es encontrar bugs, es mejorar como profesional, es transferir el conocimiento y aprender cosas nuevas. Socializar, aprender y enseñar cosas. Es mejorar el nivel del equipo en general, no importa quien haya hecho el cambio.

“the discipline of explaining or ‘discussing’ your code to your peers”

Que generan los code reviews?

  • increase team awareness
    • en que están trabajando mis compañeros
  • knowledge transfer
  • finding alternative solutions

Rules of engagement

  • as author
    • es muy importante definir el contexto
      • esto se relaciona muy fuerte con como definimos los {{commits}}
  • as reviewer
    • ask, don’t tell {{socratic-method}}
      • ejemplos
        • did you consider?
        • what do you think about?
        • can you clarify?
      • que sea una pregunta deja la puerta abierta para que cualquiera, tenga el senority que sea, puede preguntar y aprender.
    • negative bias en comentarios escritos
  • be positive
  • todos tienen que participar, no es algo que developers con mas experiencia hagan para los developers juniors.
    • mejores desarrolladores
  • forzar discusiones técnicas
    • esto genera mejor código (discussion = better code)
  • nadie tiene la palabra final, todo se puede charlar y llegar a un acuerdo, que va a permitir a todos los involucrados APRENDER
  • conflict is good, good debate
  • en caso de no aceptar una solución y entrar en “conflicto” es importante saber por que tenemos ese “conflicto”
    • es por que la lógica es incorrecta?
    • es por que no me gusta como quedo el código?
    • es por que no nos gusta el proceso?
    • es por que no acepto los comentarios?
  • los “trade off” son imposibles de eliminar, vamos a tenerlos, tenemos que definirlos y aceptarlos
  • es importante no tener esa idea de que “alguien” con mas experiencia lo aprobó entonces debe estar “bien”, es necesario hacer preguntas si lo creemos necesario, si algo no esta claro.
    • en este🡭 video, {{Elon Musk}} habla sobre hacer los requerimientos “less dumb” pero también explica que es “peligroso” que una persona mas “inteligente” te de requerimientos por que es probable que no los cuestionemos y los aceptemos sin preguntar.
  • agree to disagree
  • team ownership
    • las decisiones son de todos
    • los problemas son de todos
  • healthy debate (nuestras retros)
  • los code review no son para hacer QA, son para disctuir y aprender

Que revisamos?

  • SRP (la S de solid) single responsibility principle
  • nombre de variables, funciones, etc (naming)
  • complexity
    • can you clarify this piece of code?
  • test coverage
  • formato (sería lo ultimo), estilo de código
    • trabajar en un codebase que sea visualmente correcto da mucho placer
    • da la sensación de que todos estamos en la misma página y avanzamos a un mismo lugar
    • pero, hay un estudio de Microsoft que dice que un comentario en un PR sobre estilo, se ve negativamente, se percibe negativamente
    • para eso se adopta un style guide
    • se deja a herramientas externas que manejen eso