An all too common occurrence that slows us down in the async world is
you make a PR, wait a day to get reviews
reviews come in, your code is good and works, but there are suggestions to make it better.
you refactor the code, re-write tests, etc, ask for reviews again, wait another day again.
This can go on for two or three iterations. My suggestion is simple, if it works decently well, you merge immediately, and create another WIP PR to make improvements (you have to create the PR and start working on it, otherwise it's just tech debt). Your new PR will include all good changes suggested by your team.
This will do two things.
1) It allows the code to reach the user and deliver benefits faster.
It may even validate/invalidate the entire feature. It may reveal a wrong approach, etc. Code on the IDE provides little information, code on prod reveals more information. The faster you receive information, the more feedback you have, the more agile you are. In the traditional sense of the word agile
2) Your next PR, the one fixing the tech debt, will be smaller, and different from the first one. It'll be easier to read and review. Sometimes you had good working code, and a well meaning suggestion could break your code or introduce hidden bugs. In that case, your original code is already on prod and working, and this single PR (the one adding the suggested changes) can be reverted.
Aim for fast feedback, increase surface area. Godspeed
Top comments (5)
You are absolutely right. Not to mention that sometimes refactorings and improvements in the way things are done can hide the main feature amidst all the changes, making it more difficult for a reviewer to understand.
In many cases, I feel like the first review, although boring, might get a lot of small problems in complex systems with a lot of overlap. Hence, I tend not to enjoy the suggested approach, although I feel like a PR with the fixes not of business logic problems but coding patterns, improvements, and so on is an amazing idea.
Of course, it can change depending on the project and company, I say this since I believe in previous companies I worked at this idea would be amazing
Tem algumas coisas que gostaria de ser mais específico
O que seria "if it works decently well", existem mil maneiras de fazer um omelete, você pode ser mais eficiente, menos eficiente, sujar mais panelas, enfim...
Neste ponto você tem razão
A visão que eu tenho um Pull Request é igual a um pedido que foi feita no restaurante, tem que chegar na mesa do cliente o mais rápido possível, entrentanto, os melhores pratos seguem padrões de apresentação, trazendo pra código, o código deve seguir tanto o padrão das boas práticas, como o padrão da equipe em geral.
Eu confesso que acho muito arriscada essa abordagem, pois também, você parte de uma premissa que todos do time tem um mesmo nível, e que não vão fazer códigos tão ruins nesse primeiro merge, coisa que pode não acontecer.
Prefiro a abordagem de revisão e tests primeiro, em sandbox, seguindo boas práticas, adotando padrões, do que revisitar uma coisa(claro que há uma deadline pra isso), e tem que haver um bom senso na questão do code review também.
Mas bom artigo Augusto!
I agree with your point, but in my experience, it's better to take action in the moment rather than postponing it, because 'another time' often never comes. I believe it also depends on the team's culture.
If "in the moment" is actually in the moment, and not another cycle of 1 day of waiting for reviews, we agree :)
But it depends, async sometimes means different timezones, etc. We might want to move sooner rather than later. But yeah, if you can hop on a call and do it immediately, do it. If it'll take a day or half a day, I say create another PR, then merge the first one and focus only on the second