4 ms·

I'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 collabo

by jseban 5y ago

I'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 ago
There 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 ago
  Yeah 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 ago
    Just 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 ago
    If 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 ago
    I'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
    and yet many people on this subthread alone have worked in review-required jobs, and have not had a problem with it. They've also provided reviews for other people's patches, and presumably were also able to do it without personal attacks. It's the bare minimum of professionalism. If you are unable to separate feedback on your work, from attacks on your person, you are lacking some fairly fundamental skills needed for professional engineering.
    - leaflets2 5y ago
      > If you are unable to separate feedback on your work, from attacks on your person, you are lacking some fairly fundamental skills Or for any job role at all?
  - jen20 5y ago
    > nobody can completely separate criticism of their work from criticism of themselves Yeah I really hate those pesky automated linters running in standardised environments telling me I’ve screwed something up. (/sarcasm, hopefully clearly!)
  - sethammons 5y ago
    In over a decade of enforced code review experience, I've had one developer who was too immature to take feedback. Some folks take it personally and they shouldn't as long as the feedback is about the code. This requires some work on both reviewer and reviewee. The guy who couldn't take feedback (person A) had code merged in that wasn't properly tested. Person B said, "hey A. I could use some help. We wrote some tests around $feature that were missing and the tests show the feature doesn't work. We see $unexpected results. Wanna make sure we agree on how this should work." Person A: no, it works, I tested it. B: could you help us identify the flaw in the tests then - they seem rock solid. A: no, my code works. B: ... ok, can you join us and talk through it? A: no, it works. A was removed from the team after management came in and A continued to not acknowledge his code could be wrong. This was aberrational. We, as an org and as a team, constantly strove to keep the focus on the quality of the code. And, yes, his code was borked.

by jseban 5y ago

actually_a_dog 5y ago

There 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 ago
Yeah 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 ago
  Just 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 ago
  If 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 ago
  I'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
  and yet many people on this subthread alone have worked in review-required jobs, and have not had a problem with it. They've also provided reviews for other people's patches, and presumably were also able to do it without personal attacks. It's the bare minimum of professionalism. If you are unable to separate feedback on your work, from attacks on your person, you are lacking some fairly fundamental skills needed for professional engineering.
  - leaflets2 5y ago
    > If you are unable to separate feedback on your work, from attacks on your person, you are lacking some fairly fundamental skills Or for any job role at all?
- jen20 5y ago
  > nobody can completely separate criticism of their work from criticism of themselves Yeah I really hate those pesky automated linters running in standardised environments telling me I’ve screwed something up. (/sarcasm, hopefully clearly!)
- sethammons 5y ago
  In over a decade of enforced code review experience, I've had one developer who was too immature to take feedback. Some folks take it personally and they shouldn't as long as the feedback is about the code. This requires some work on both reviewer and reviewee. The guy who couldn't take feedback (person A) had code merged in that wasn't properly tested. Person B said, "hey A. I could use some help. We wrote some tests around $feature that were missing and the tests show the feature doesn't work. We see $unexpected results. Wanna make sure we agree on how this should work." Person A: no, it works, I tested it. B: could you help us identify the flaw in the tests then - they seem rock solid. A: no, my code works. B: ... ok, can you join us and talk through it? A: no, it works. A was removed from the team after management came in and A continued to not acknowledge his code could be wrong. This was aberrational. We, as an org and as a team, constantly strove to keep the focus on the quality of the code. And, yes, his code was borked.