4 ms·
This strikes me as somewhat un-pragmatic. Non-blocking could cover everything from “wow this is kind of jank, but it’s good enough for now and let’s figure out
by FridgeSeal 1y ago
This strikes me as somewhat un-pragmatic.
Non-blocking could cover everything from “wow this is kind of jank, but it’s good enough for now and let’s figure out how we fix it later” through to very minor thing like “you have a typo”.
If we blocked on everything being perfect, we’d never get anything finished. Plus, PR’s are free.
- dns_snek 1y agoHow do you organize that information so that you can actually follow up on it later? Writing a non-blocking review feels a bit like writing to /dev/null. Good to get it out of my system but not much else.
- 1718627440 1y agoPut it into a comment or into one of todo.txt, features.txt, ideas.txt, under docs/ or in README?
- dns_snek 1y agoWhat's the point of leaving a review comment then?
- 1718627440 1y agoBecause it belongs in to the PR and only the Author knows the answer: his thoughts and rationale for the change and behaviour. Sure I could just guess, but then it's essentially poking in legacy code. The point of it being not legacy code is, that the person is still around, so he should write down his reasons, so that they are documented.
- dns_snek 1y agoSure, what I'm trying to say is that you need to design a process which ensures that these non-blocking review comments get organized properly, and enforce it. If you need the author to add your comments to TODO.md then that should once again become a blocking review.
- 1718627440 1y agoI do not want to prescribe the author what he needs to do. I only want to make sure that it was an intentional choice to not do that.
- FridgeSeal 1y agoYep, or make a ticket in whatever issue/ticket/work management system you use. Everywhere I’ve worked has always had some kind of “future stuff we’d like to do” project.
- dns_snek 1y agoBut then rather than leaving a non-blocking review, why not leave a blocking review and approve it as soon as the author responds with a link to the ticket or adds it to TODO.md? If it's important enough to be tracked then I'd want to make sure that it's been filed properly. It's too easy to forget about such a trivial task, especially in a time crunch.
- FridgeSeal 1y agoBecause we’re pragmatic, and I trust my teammates to handle things like that without the need to babysit them or block them over trivial tasks? I trust that it will get done. It is not important to me, whether it happens before the PR goes in or afterwards.