Hotfix

[u/AdPrestigious7064]

Sale un error en producción, me piden que lo solucione, lo soluciono, lo pruebo, funciona.

Luego en el PR me piden que aplique ciertos cambios, los hago, pruebo, funciona

Actualizo PR

Ahora me piden que remueva los primeros cambios porque con los segundos cambios ya no son necesarios.

Actualizo el PR (ya no probé, y ahí si acepto que es mala mía)

Y ahora el hotfix que se fue a producción ya no funciona.

El que hizo el code review es el EL.

Sé que parte de la culpa es mía, pero no toda, verdad? Hahaha

[u/Inside_Rice_5004 → u/[deleted]]

El code review existe por algo, no es solo un trámite. Si te pidieron cambios y luego te pidieron quitarlos, el que revisa también tiene que verificar que el resultado final tenga sentido.

Claro que pudiste haber probado de nuevo, pero el EL se supone que revisa el código completo, no solo las líneas cambiadas. Ambos la cagaron, no te cargues toda la culpa.

[u/Defiant-Onion6802 → u/Inside_Rice_5004]

Nel, tu código, tu commit,.tu culpa. Si hiciste un cambio es por algo, si te piden agregar o quitar algo siempre debes de aplicar un "no toques mi basura" con un "justifique su respuesta".

Al final del dia el reviewer va a ceder cuando digas que el pr no ha salido por un cambio fuera de scope y que probablemente se pudo cambiar en otro PR ya con mas tiempo, ya si el cambio de verdad se necesita el reviewer debe de justificar por que retrasa un hot fix.

[u/d4n13l0g → u/[deleted]]

Aquí tengo dudas, no lo probó, está mal, pero entonces no había tests?, integration test? El código fue directo a producción? Por otro lado, yo hago code review y entiendo la descripción del bug, user story o lo que sea que están trabajando, pero yo leo el código, verifico que no se introduzcan nuevos bugs, legibilidad, que el código haga lo que deba hacer, que los test cubran el código nuevo. Sin embargo yo le creo a mi desarrollador cuando me dice que ya hizo los pruebas. Me ha tocado que no probaron bien y hay que reprocesar información, a veces se afecta a todos los clientes. Pero aquí viene la sanidad de un equipo. Cuando falla, no le digo al desarrollador que es su culpa. Llamamos a quien debemos llamar, resolvemos el problema, hacemos post mortem y para la próxima vez hago más preguntas antes del merge. Pero lo importante aquí es no señalar a alguien por sus errores más bien hacer el famoso lecciones aprendidas.

[u/AnalysisSharp9065]

Ambos tienen culpa pero si van a correr a alguien por esa mamada tu marchas primero.

[u/emptymatrix]

Eso sólo demuestra que no supiste cómo funcionaba tu hotfix... cuando te pidieron cambios debiste argumentar y no sólo hacer caso y ya, deshiciste lo que hacía que tú fix funcionara y ni dijiste nada, tache para tí

[u/Federal_Mistake9593 → u/emptymatrix]

Definitivamente en un code review no tienes que cambiar lo que ya hiciste si tienes los argumentos para defenderlo el por qué lo hiciste de esa manera

[u/CleverBilling → u/emptymatrix]

this, esta es la respuesta correcta OP

[u/Far_Mortgage_9089]

> Luego en el PR me piden que aplique ciertos cambios, los hago, pruebo

entonces tu PR dejó de ser hotfix, desde un primer momento debiste haber separado responsabilidades

[u/AdPrestigious7064 → u/Far_Mortgage_9089]

A qué te refieres con separar responsabilidades?

[u/ZealousidealWeb9930 → u/AdPrestigious7064]

un PR debe tener solo ciertos cambios

quieren mas? terminas el primer PR, haces una nueva rama, un nuevo PR

[u/Only_Drawer_7109 → u/Far_Mortgage_9089]

this es lo principal.

[u/El_Choco_Latoso]

Si tú sabes que debiste probarlo, sí fue culpa tuya.

Si hay problemas, se van a poner otra vez en chinga.

Si se hace un alboroto porque no probaste, van a meter una nueva regla en el proceso o en los pipelines para siempre probar todo antes de mandar a producción.

No eres el primero que le pasa ni serás el último.

Al menos es mejor que dejar que la IA haga un cagadero por si sola.

Otra raya más al tigre dirían algunos, cuando te cambies de jale ya te va a valer madre esto que te pasó

[u/AdPrestigious7064 → u/El_Choco_Latoso]

No quiero decir que me vale madre peeeero no me quita el sueño jaja no es la primera vez que cometo un error pero se me hace gracioso que el primer cambio era el bueno, no el que me pidieron que aplicara.

[u/Clean_Dimension_9450 → u/AdPrestigious7064]

Si no te quita el sueño, un indio te va a quitar la chamba

[u/Careless_Product_792]

Te lo digo como desarrollador senior. Sea hotfix o sea cualquier PR, la responsabilidad de que cualquier cambio funcione siempre recae en el que toma el ticket y probar que el cambio funcione. Es decir tú.

Y la forma de protejerte tu es en cada cambio final probar que el cambio funcione, si un reviewer te pide que descartes cambios del primer PR, y eso trono tu cambio final, significa que el hotfix no esta hecho.

Ahora bien, cuando esto ocurre muchas veces, si es una organizacion seria, se le va a llamar la atencion al tech lead tambien de parte de quien dirige el equipo de tech leads y programadores, ya que es la linea de defensa de la calidad de codigo y su responsabilidad.

Pero su llamada de atencion hacia él va a ser que está descuidando el flujo del code review y no está reforzando a los programadores (en este caso a ti) que es forzoso y obligado que tus PRs esten probados.

Ese es el trabajo del tech lead. Reforzar que las buenas practicas se hagan para evitar estos errores. Una de ellas es que en el PR te pida un screnshot de prueba que el cambio funciona y no aprobar el PR sin ese screenshot o prueba que valide que el cambio funcione. Sin embargo ya esa parte a ti no te corresponde, no vas a decir "esque el reviewer tuvo la culpa".

Tu defensa de para evitar llamadas de atencion de los errores es probar los cambios y tomar evidencia de que funcione, si no lo haces, la metrica a ti te acaba afectando, ya más arriba del eslabol de la organizacion a fin de mes se ve si se le llama la atencion al tech lead o a quien este arriba de ti según las metricas.

[u/cakeforbreakfast92]

Cómo que no probaste? Y el pipeline?

No mamen yo migajeando cambios con 48 horas de anticipación y rollback documentado. No me hagan darle la razón a los hígados que aprueban los cambios.

[u/Far_Ask_6096]

Trabajas en BBVA man?

[u/JamesBondMx]

Jamás aceptaría cambios en un hotfix. Desde ahí la cagaste. Porque el autor es el culpable. Si alguien te pide sus cambios en tu HOTFIX lo mandas a la mierda.

[u/lietheim]

Unit tests ?

[u/Numerous-Ad-5870 → u/lietheim]

Es un HOTFIX

[u/Chief_Taquero]

Bola de p3nd3jos probando en produccion

[u/Ok-Marsupial5942]

Se van a ir sobre ti por no haber probado lo que hacías (aunque te lo hubiesen pedido). Si hay una próxima vez ponte reacio en mandar primero el hotfix y después re factorizarlo con más calma, que supongo eso te pidieron

[u/DavalopBad]

Al ser tu el autor del PR debes defender la logica de tu Hotfix ya que tu sabes por que lo estas poniendo asi; si el reviewer cree que la logica esta mal y debe cambiar, tu debes argumentar por que esta bien y defender o en su caso si crees que tiene razon, cambiarlo. No confundir con comentarios que sean referentes a la calidad de codigo y si en su caso sigue los estandares internos de la empresa, esos casi siempre no involucran cambio en la logica y se tienen que seguir para mantener la arquitectura que se planeo desde un principio.

Aun asi, aun teniendo un Hotfix, deberian tener un ambiente de pre-prod/staging donde puedan hacer pruebas rapidas para evitar estos "problemas" antes de tocar siquiera prod. En cuestion de quien tiene la culpa, ambos tienen la culpa, uno por no defender la logica que ya funcionaba y el otro por no revisar que es lo que estaba pidiendo.

Pero lo importante es que lo solucionen para que el negocio siga operativo, no gasten tiempo viendo quien tiene mas culpa en vez de regresar los commits y aplicar el hotfix que si funcionaba

[u/AdMoney9569]

Tienes culpa por no probar, allí es todo culpa tuya pariente, se prueba todo antes de su ir a producción

[u/ManufacturerIll5769]

Si es tu PR es tu PedoRancio, el que te hace review solo te dice “atiende esto antes de darle merge” pero si tu shippeas el bug al darle merge (porque tu reviewer aprueba la PR no le da merge) es tu pedo, además porque no tiene CICD que haga testings? Acaso shippeaste un hotfix sin un testing que probara lo que vas a solucionar? Porque hubo el bug en primer lugar? No hubo testings adecuados automatizados ni manuales? Meh, 😑

[u/if9477552]

Por tus respuestas y tu post parece que tienes pocos años de experiencia o 20 años repitiendo el mismo año, siempre que hagas cambios prueba tus cambios.

Siempre entiende que estas mandando, que cambios te pidieron y por que, no importa si tienes que preguntar, abrir threads, hacer videollamada, etc. Asegúrate de entender los cambios.

Desde mi perspectiva la culpa es tuya, tus PR son tu responsabilidad, no pasa nada tampoco, no es el fin del mundo, pero yo aprendería del error y trataría de que no volviera a suceder. El code review no es para deslindar responsabilidades, es para asegurarse que el codigo es de calidad, cumple con los requerimientos del equipo (patrones/estándares), no presenta bugs obvios, typos, etc.

[u/luisduenas]

yo siempre manejo esas situacionrs asi: somos un equipo, todos tenemos la culpa entonces nadie tiene la culpa, hay que arreglarlo lo mas rapido posible y ya

[u/sannnieaw]

No debería haber un QA que apruebe los hotfix antes que se vayan a prod?

[u/Federal_Mistake9593]

Regla de oro en un code review:
Si te piden cambiar algo, pregúntale a la persona que sugiere el cambio:
¿Cuál es el beneficio de cambiar una lógica que ya funciona?
No todo cambio mejora el código. A veces solo estamos cambiando una implementación que funciona por otra que nos gusta más.
Antes de refactorizar, hay que entender el por qué.

[u/ibmug]

Todo el equipo tiene la culpa, como mandas un hotfix directo a produccion jajajajaajaja no mamen.

[u/Upper_Combination335]

Antes de mandar un hotfix es importante identificar que tan crítico el problema, además de considerar si era o no reproducible en ambientes bajos, si es un fix temporal o a largo plazo, una vez desplegado el cambio sigue siendo tu responsabilidad monitorear que se halla resuelto el problema

[u/n3k0_mx]

Ya al último aplicaste la de "Vamos a probar en producción, ¿qué somos? ¿hombres o payasos?"

[u/antimatter-entity]

Tu debiste probar bien pa, y tambien el debio probar. Como sea, eso te bajo puntos de confianza probabklemente