The trouble with the PR model is that it is done at the end of the development cycle for that feature/bug fix.
What if the result of the code review was more important than "why did you format the code this way? Why did you name that method XYZ? ABC would be better to represent its true purpose" - case A.
What if the result of the code review was something like "why did you write that entire functionality from scratch when we already have 95% of it already implemented in production proven classes?" - case B.
Trust me, I've experienced too many "case B" OMG moments to dare to think about.
The PR model is great for situations like case A but is completely inappropriate for picking up major architectural issues like case B. If architectural issues are picked up by a PR then it's probably at the end of a sprint and at that stage it's waaaaay too late for the dev to go back and rewrite the code in a way that doesn't screw up what might have previously been a good, lean architecture. So the pressure is on to accept the PR and commit the company down the path of unmaintainable bloatware with a crappy architecture.
When you kick up a stink and try to knock back PRs based on attempting to promote "maintainable code for the future viability of the company" other devs and project managers frame you as some kind of "productivity blocker".
Eventually junior devs are scared to ask you to do code reviews and then further down the track, incrementally, sprint by sprint the company's software becomes the bag of crap you predicted it would become and the team is under pressure from management because users start complaining that the software is slow, unreliable, annoying to use etc.,
What's more important is a design review before devs start writing the code to implement a feature or fix a bug. Maybe they should have to present to the group what they're planning to do before they run away to their rabbit hole and reappear 7 days later with a big bowl of spaghetti that, due to the PR model's 'end of sprint' scheduling, someone will be pressured into accepting... so that "we can get it into testing before the sprint ends".
Insisting on a "Design Review" (DR) before implementation means that a key function of the "Pull Request" (PR) at the end would be to confirm how close the dev's implementation is to the agreed upon design. i.e. not just some relatively unimportant "feel good" B.S. to make sure that their "opening curlies" are the right place.
The trouble with the PR model is that it is done at the end of the development cycle for that feature/bug fix.
Waiting until the end of the development cycle is indeed a bad idea.
Our team tries to do various forms of pairing on a feature/bug before we ever get to a pull request. We also encourage opening them very early in the development cycle rather than waiting until the end.
Thanks for reading!
For further actions, you may consider blocking this person and/or reporting abuse
We're a place where coders share, stay up-to-date and grow their careers.
The trouble with the PR model is that it is done at the end of the development cycle for that feature/bug fix.
What if the result of the code review was more important than "why did you format the code this way? Why did you name that method XYZ? ABC would be better to represent its true purpose" - case A.
What if the result of the code review was something like "why did you write that entire functionality from scratch when we already have 95% of it already implemented in production proven classes?" - case B.
Trust me, I've experienced too many "case B" OMG moments to dare to think about.
The PR model is great for situations like case A but is completely inappropriate for picking up major architectural issues like case B. If architectural issues are picked up by a PR then it's probably at the end of a sprint and at that stage it's waaaaay too late for the dev to go back and rewrite the code in a way that doesn't screw up what might have previously been a good, lean architecture. So the pressure is on to accept the PR and commit the company down the path of unmaintainable bloatware with a crappy architecture.
When you kick up a stink and try to knock back PRs based on attempting to promote "maintainable code for the future viability of the company" other devs and project managers frame you as some kind of "productivity blocker".
Eventually junior devs are scared to ask you to do code reviews and then further down the track, incrementally, sprint by sprint the company's software becomes the bag of crap you predicted it would become and the team is under pressure from management because users start complaining that the software is slow, unreliable, annoying to use etc.,
What's more important is a design review before devs start writing the code to implement a feature or fix a bug. Maybe they should have to present to the group what they're planning to do before they run away to their rabbit hole and reappear 7 days later with a big bowl of spaghetti that, due to the PR model's 'end of sprint' scheduling, someone will be pressured into accepting... so that "we can get it into testing before the sprint ends".
Insisting on a "Design Review" (DR) before implementation means that a key function of the "Pull Request" (PR) at the end would be to confirm how close the dev's implementation is to the agreed upon design. i.e. not just some relatively unimportant "feel good" B.S. to make sure that their "opening curlies" are the right place.
Waiting until the end of the development cycle is indeed a bad idea.
Our team tries to do various forms of pairing on a feature/bug before we ever get to a pull request. We also encourage opening them very early in the development cycle rather than waiting until the end.
Thanks for reading!