What I Think is My Relevant Background
Part of my job is the ensure the quality of the code deployments to the production environment. This is my first job where I kind of review at such a low level. I've been with three different companies as "release manager" (whatever that means) over 10 years.
Before that I was a software developer for over 10 years.
Although we are the technology arm of my current company, our company is not a technology company. Our system still runs classic ASP and "upgrading" to .NET 4 for many years. This "new" tech has not been supported by Microsoft for quite some time. Why don't we just upgrade to .NET Core? The answer to that remains unknown. I bring this up only to say that the internet is full of anecdotal examples of decent to best-in-class developers. A lot of the confusions to modern methodologies is that there is a large diversity of experiences in software development and why there are so many opinions on things that seem to logically work but don't.
Our Process
I probably should start on what code review is for us but I am going to over-simplify by just saying that someone kind of looked at it and there is some sort of checklist they should go through. Our process requires someone to at least sign off on the code review. There is no consequence if bugs are found or rolled back.
So for us, I found that they always give the code review to the newest member. And they always approve the code review. I don't think I have seen one finding from any newly hired developers. If there is a database code change, then it also goes through db review and possibly db performance review.
The code is also deployed to pre-deployment environment for "system end-to-end" testing but it is actually all testing because everyone is too lazy to figure out how to create test data for there changes. This deployment has now been delegated to other lead developers because I take too long to deploy changes. Or rather, I cause delays in the development process... because I care about quality?
Who Reviews our Code?
It always seem to be the newest person. Even after our massive layoff, that just went back to the prior newest person. Not only that, developers hate having their code reviewed. Because for a period of time, they always put in comment that "change was minimal, review now required." Yet rollbacks increased, and they never changed until I told leadership that seems to be the trend. Then magically, rollbacks decreased again after code review.
Does our code review work? I still have my doubts. I think it just forced the senior developers to actually slow down or face negative optics.
The amount of low quality development and review is mind-boggling to me. I look through code like a resume, for about 5-7 seconds. The amount of findings like incorrect dates, incorrect reference number, incorrect date format, misspelling of object names... is just astounding to me.
After seeing how little management or leadership cares about the consequences, I no long push back on many of these "minor" changes. I almost only push back when they update a generic library that I have not seen modified for 4 years suddenly needed for a specific function for a specific customer. This still blows my mind as no one (about 4 pairs of eyes) even questioned that before it reaches to me.
There was also a change to update a library to add a connection string to the primary database that the entire application has been using for the last 10+ years. How did no one question this? First, why is the connection string with user name and password hard-coded into a library. Second, no one bothered to think how did everyone else access the primary database over a decade. Do I even need a third? A second was probably not even needed.
Who Should Code Review? Jr or Sr?
In my opinion, everyone but should be a trusted resource that signs off on the code review. An untrusted code reviewer should review first then a trusted resource.
Technically from a release manager's POV, I only care that someone signs off. But as a dev lead, I would prefer multiple reviewers until all developers are trusted resources. Once everyone is trusted, then one reviewer is sufficient.
How do you know if someone is trusted? This all depends on the quality of the deployments. If too many, then the person is untrusted. What is too many? That depends on the company. If the company has a high tolerance for unexpected downtimes, then it is not as critical. If the company deals can impacts a person's life, then it is important a person has a record of good performance and possibly more safe guards to manage regular human errors.
No comments:
Post a Comment