I've created a code review for merging someone else's code, coz they were signed off sick for the last month.

They're making comments about how it's wrong.

It's code they wrote, but restructured to be more readable.

They wrote incorrect code that was just so illegible they didn't realise.

How do I explain this diplomatically?

  • 2
    I don’t get it. How is it wrong? Do you have some weird work mentality where pieces of code belong to someone?
  • 2
    @Lensflare they wrote it, I don't really get what it does but the tests still pass. It was just pretty messy before, now it's simpler and more legible.
  • 2
    I guess say "thanks I'll get that fixed."
  • 5
    @atheist In general, if you don't get what it does, you can't change the order of statements. Tests only cover the things you break when the test writer invested more time into the tests than into the tested code. So don't trust other people's tests (and probably your own ones neither).

    But if you understood the code and then refactored it for maintainability, that is fine. And you can just tell em that you did. Normally, you don't need to be careful about that.
  • 1
    @Oktokolo that's actually useful advice, thanks. I have refactored the code and maintained equivalent behaviour, they think I've added something that was already there. I understand the code in the sense of what it does logically, not in the sense of what it's intended to do business logic wise. They're asking for benchmarks and such, but there's no point because nothing has changed.
  • 0
    @atheist Maybe revert back to their original code and start adding your own comments. Point out the flaws in the code that you previously addressed and ask them to clean it up and provide benchmarks.
  • 0
    @Cvrsor the project is 3 months behind. I'm not saying they're incompetent, but I tend to get given problems that other people don't know how to solve...
  • 1
    I got an apology today about how this was all handled. Progress!
  • 0
    Benchmark is in. My "refactored code" is 2x faster. Binary identical output. Fuck those people.
  • 0
    I'm now going to slack off for the rest of the week.
Add Comment