4 ms·
> Review also shouldn't be causing ego antagonisms. Yeah but the tool itself is antagonistic, because it imposes a workflow of open source, a workflow that als
by jseban 5y ago
> Review also shouldn't be causing ego antagonisms.
Yeah but the tool itself is antagonistic, because it imposes a workflow of open source, a workflow that also is antagonistic with clear and absolute power. So using that tool suddenly brings that antagonism and power into a team which is supposed to be 100% collaborative, and it also only does it periodically and with random and different people in power. It's not hard to see how that becomes somewhat complicated.
- actually_a_dog 5y agoAnyone who can't handle having their code reviewed before it goes into production is not welcome on my team. You can call that antagonistic or not, but I have always found that the best engineers are the ones who are able to separate criticism of their code from criticism of themselves. Get that type of people on your team, and code review is no longer antagonistic: it's just an egoless feedback process that improves the end product, which is what all members of an engineering team should be working toward.
- jseban 5y agoI'm not criticising the review itself as much as rather the PR workflow and the tools of github, especially the blocking mechanism. It's my opinion that collaboration should happen much sooner rather than being pushed to the end with the review of a PR, and that you should have designated people who have the power to sign something off as production ready. If a team inside a company want to gather inspiration from the open source world, they should look at how the owner teams of open source projects work internally, not how they work together with outside contributors. And, it's a very common "solution" in tech to just simply not have any feelings, but people have feeling and that you can't turn off, and that's a very important contributor to work satisfaction.
- actually_a_dog 5y agoThere is nothing about github hat prevents you from working that way. I don't see what your issue is. But, I will say that I don't want people who can't separate criticism of their code from criticism of themselves on my team. You can certainly have whatever feelings you want, as long as it doesn't get in the way of producing the best possible product, all things considered.
- jseban 5y agoYeah there's nothing that prevents you from using a tool in exactly the opposite way that it's designed to be used, but it's also pretty unlikely that it's going to happen or that it's going to be successful.
- Jensson 5y agoJust make a PR early and discuss the code as you build and make changes, not sure in what way the tool wasn't made to do that. Then you also get the discussion interleaved with changes, resulting in basically perfect documentation of how the code was made and why. The only thing PR's lack in that regard is that if the reviewer accepts it then it gets added automatically, while ideally you should re review things after the reviewer has accepted it. That way they wont accidentally accept prototype changes.
- forgotusername6 5y agoIf I want early feedback on something I create a draft PR and ask for comments. I can then let people consider my approach asynchronously, in their own time.
- actually_a_dog 5y agoI'm beginning to think you've never actually used github. Are you trolling? The tool does not stop you from working in the way you suggest. Maybe it's not what the engineers who wrote it originally envisioned, but it's both simple and easy to work in the way you suggest.
- jseban 5y ago> I don't want people who can't separate criticism of their code from criticism of themselves Yeah good luck with that, nobody can completely separate criticism of their work from criticism of themselves. You are making your job as the team leader way to easy for yourself, "I only hire robots, that's how I solve all these pesky people issues".
- olliej 5y ago
- olliej 5y agoWhat tool are you talking about? I've had patch review as back and forth comments in email, in bugzilla, in myriad other bug databases. If you can't send out an email with your patch as an attachment, and get feedback, then we have a problem, and the problem is not the adversarial nature of review.
- jseban 5y agoBut why would you use such a remote asynchronous late stage feedback loop, if you are literally sitting in the same room as your collaborators, during the whole development process?
- olliej 5y agoYou're not understanding the purpose of review. The goal in review is to try to catch any oversights or errors in the code you wrote. The code you wrote may have been the result of of discussions with your coworkers, but the code still gets reviewed, even if by people you discussed the implementation with. That review will occasionally find things you missed, and then you can cycle, and re-review. By definition, the review can only happen at the end of change development.
- jen20 5y ago> if you are literally sitting in the same room as your collaborators Your very premise is wrong. At any sufficiently large company, you are unlikely to be sitting in the same room as every collaborator and stakeholder, or even proxies for them. As a simple example, the team I currently work on (on one project of several) is 10 people across 8 US cities in all four US mainland timezones, and the stakeholders and collaborators are across Australia, Asia, Europe and the Americas. A good majority of what I do is pair programmed, yet the pull request workflow is essential to letting _others_ know what is happening and why, and to allow them to have asynchronous input into the process. You might argue it would be better for the team to be in a room somewhere. Maybe so, but the people this project demands live where they live and could not even agree a common location for that room to be if they wanted to. And it still wouldn’t help the other projects…
- lovich 5y ago>So using that tool suddenly brings that antagonism and power into a team which is supposed to be 100% collaborative, and it also only does it periodically and with random and different people in power. Is code review not collaborative in your experience? In every team I've been on/run we've split code review comments between suggestions/questions and actual "I will not let this go into prod if my name is attached to it" blockers. Code review has been 99% the first set of comments and only rarely do I see anyone actually block reviews over things like style and what not that have been mentioned here. I can't even imagine those topics getting into code review as a blocker as if we have actually strongly held opinions on mechanical issues like that, they are integrated into the various linters used so you don't even need human eyes to catch the issue.