Despite declining these requests and suggesting alternative solutions, such as recommending another engineer with more familiarity with the feature, the web team still approves these large Pull Requests. How can I effectively communicate my concerns about the code quality and potential issues or bad coding practices within these large PRs, and what steps can I take to ensure that the appropriate team members are reviewing and approving them?
Context about me & external team: I'm a senior engineer and one of the code owners. I'm responsible for backend changes for API, but I am often asked to review Pull Requests coming from different changes also for Web (React, TypeScript). I noticed a lot of Pull Request coming to me since our codebase is 1 large monolithic application that powers web, mobile, and backend platform services, and it is stored in 1 GitHub Repro.
The external team are people who I do not work directly with and they are new to our codebase, but their team is pushing out new features as new major initiative from another Team and the stake are high for their project to be roll out. Lately, there's multiple bugs introduced from the external team, and it causes Production issues. And I'm getting buy-in from my Directors that we need to lay down some rules for changes from the external team.
Hey there! This is a great question I'd love to weigh in on. I know this got partially answered during office hours so I'll reiterate for posterity + add some thoughts of my own.
During office hours (and I 100% agree):
Here's what I'll add regarding communication:
That last bullet spiraled out of control a little bit, but I just wanted to nail in the mindset of the collaborative relationship, and you are just looking for a process or way that you two can work together that will benefit both of you.
Hope this helps! Best of luck.
Here's what I would do in your situation:
I definitely 100% agree with the previous answers, however, I am also interested to find out why weren't the bugs caught in pre prod environments. Obviously, the huge PRs make it difficult to review code, but even then, I don't think the responsibility of catching bugs is on the code review process. If the code deploy pipeline does not have enough integration tests or the repo itself does not have enough unit tests, this is a good opportunity to invest time in writing those. Apart from enforcing a process in PRs, there should also be focus on improving the code deployment process. Practices like integration tests in pre prod, canaries in prod, and proper instrumentation for catching anomalies earlier on will also help in this scenario.
And, though it won't help your situation immediately, I would strongly recommend to carry out a repo split into multiple more manageable repos. Since you are a senior engineer, you should try to convince your team and management to invest in such an effort, as this will simplify ownership and also promote the velocity of roll outs in the future.