How to deal with a manager who doesn't follow process

Question

I'm a software developer and we've recently implemented code reviews. The process should go like this:

• The developer submits a pull request.
• The developer's partner reviews the code, and approves it/leaves suggestions
• Once the partner has approved it, one of our two managers/admins approves it and it can then be merged into our main branch

The problem is, as we near our deadline, one of the managers constantly removes me from my partner's PR review lists, approves it, and merges the code in without reading it. My partner was hired as a senior developer (but has worked on this codebase for less time than me) but based on what I've read/questions asked writes infinite loops and is unaware how classes are instantiated. If I'm not able to get to the code review within an hour or two (which I don't find unreasonable), I will often then see that it has been approved/merged without my input. This obviously causes issues down the line.

How do I approach my manager about this situation without seeming like I'm attacking the other more senior developer?

**Non technical version: ** We have a process in place, which my direct manager often ignores because the other person hired was more senior and he doesn't check her work, even though it is lacking. How do I deal with this?

Answer 1

Few points to convey to your manager and team mate:

1. Quality: You should be the voice of quality when you talk to your manager. Even if a deadline is approaching, poor processes only lead to poor quality

2. User error / Knowing ones code too well: This can very well cause even a senior developer to make silly mistakes in the design approach and even sometimes basic Java practices around writing code. Thus, it is a must that at least 1 more pair of eyes go through the code change so that such things can be identified and fixed.

3. "We have a process in place, which my direct manager often ignores because the other person hired was more senior and he doesn't check her work, even though it is lacking.": Seniority can cause this kind of an attitude in people, which is a different battle, but the least you can say is that another senior developer be added to your colleagues code reviews if not a junior (don't mention you as the one being junior. That may sound as though you are taking this personally).

If none of this works, don't fret. It's important you keep up your good work and don't become a person who disregards processes. You follow them. All the best!

Answer 2

Do something radical and talk to your manager, without accusing anyone of anything. "Hey, I see you approved X, Y, and Z. I thought I was responsible for those -- did you take them because there's something I need to get better at, or did you just have some spare time to work on them? We should coordinate this better to make sure we aren't wasting resources by having unnecessary parallel reviewing."

Then listen. Carefully. Whether you agree or not, what you're told is going to he the actual process as implemented. It may not be the one you're assuming. In particular there may be no ownership of reviewing tasks; it may be assigned to you to make sure it gets done, but others may be actively encouraged to help out when time permits.

Remember that the system tracks approvals. If your manager says code is OK and it isn't, you will not be blamed for that; management and the other coder will handle it appropriately, and one or both of them will learn from the experience. The system is working; let it work.

As far as your doubts about the other people's work goes... we can't tell from here if you are right about your concerns or not. I suggest pausing for a moment to consider that just maybe your manager did review the changes befire approving them, and that maybe your problems with the other coder's work are more a matter of misunderstanding and of different styles of approaching the problem rather that their code necessarily being worse. Maybe that loop isn't infinite after all, but merely exits differently than it would have had you written it. Or maybe it was, but everyone makes a mistake once in a while and you shouldn't assume they will continue to make that mistake

Don't tell people they're wrong. Tell them you're not sure you understand, and ask them to explain why they're right. Even if they aren't -- especially if they aren't -- this is a far more polite way to open the discussion. And it keeps you from embarassing yourself if they do have a good answer.

Don't contend. Collaborate.

Answer 3

The reviewer has the responsibility to make sure that the code is of acceptable quality. That's what you would make sure of if you were the reviewer. Since your boss is the reviewer, it's his responsibility that your co-workers code is of acceptable quality when checked in, not your responsibility, and not your co-worker's.

That needs to be made clear to the boss. If you end up shipping a product with severe defects, and it costs your company money, then you are not to blame, nor is your co-worker (even if his code caused the fault, because the real cause of the fault is the incompetent review), it's your bosses fault entirely.

Senior or junior doesn't make a difference. People make mistakes, whether junior or senior, just different mistakes. Reviews catch them. If the boss decides to do incompetent reviews, any problems coming from this are his fault.

Answer 4

Not sure you want to try too hard to manipulate your manager. He has decided to take the risk and let potentially bad code go into production. Who knows, he could get lucky.

When you are required to debug, enhance or alter the code, you should consider padding your estimates on code you haven't reviewed because in a way, you need to do the review now.

Again, your boss will have to decide how to address the extra time you're taking. I would make sure you document these situations. I don't know if you want to get into any details on the quality of the code or lack of skills by the other developer. If your boss feels threatened by all this, he'll make excuses to defend his actions and the capabilities of the other person and put more blame on you or just tell you to stop taking so long (i.e perform magic).

Maybe the manager will see the error of his ways or at least the other programmer.

Answer 5

I feel like there are a couple of issues with this approach...

Pick your battles.

I understand how annoying it is when someone else you work with writes bad code. It makes everyone else in your department look bad because end users tend not to understand the difference between a bad UI and a good backend, for example. That being said, you really ought to review with yourself whether or not this particular issue is worth spending relationship capital with this supervisor (or whomever else you have to turn to). Along the same lines...

Try to keep your circle of concern within your circle of influence.

Sorry to get all Stephen Covey here but I think this applies. According to the rules as written, you have a distinct vertical in place. You are supposed to review this other guy's code, but when someone else comes by and marks it off as approved, you no longer have either the need nor the authority to review said code anymore. It's outside of your circle of influence, and hard as this may sound, you kind of need to push it outside of your circle of concern as well for the sake of your own sanity.

If the system as is happening is failing right now, consider allowing it to fail once or twice.

Consider this: your partner puts something into the code that you know is going to eventually cause your solution to crash (I'm a little bit skeptical of the "infinite loop" thing that was proposed in the comments, as most modern IDEs won't even compile if you do that, but certainly the general concept of putting out untested/sloppy code exists) and said code gets approved and follows the path to production. What happens when the bug is caught? If people are coming to you asking how you let it through, you should have a chain of evidence that says that you did not, in fact, touch or otherwise approve said code.

On the other hand, if it ain't broke, don't fix it.

I've also run into some pretty damn inflexible developers in my day to be honest. What is "broken" to one person is simply "another way of doing things" to another. If you saw a bug and the bug never actually fired, is it really a bug, or did someone else just come up with a different style of fixing the issue than you did? If it performs worse than your way, then that may be an opportunity to say "well, I would have done this like X", but the reality of programming is that there is rarely only one proper way to fix things.