Office Life

Your teammate approves bad code and it gets merged. What do you do?

Jan 9

Teammate merged some bad code yesterday. I just read the PR and it’s pretty bad. Basically repeated the same code for each model type on each view.

God damn. These rubberstamp ✔️🤦‍♂️

comments

Want to comment? LOG IN or SIGN UP
  • Amazon / Eng:!
    Do the needful and revert back
    Jan 910
    • Amazon / Eng:!
      Sorry for not being kindly
      Jan 9
    • Snowflake Computing Bullshitty
      No problem. You did the needful.
      Jan 9
    • Autodesk senator
      Can we stop with this weird subtly racist joke. It’s not funny
      Jan 9
    • Snowflake Computing Bullshitty
      Racist?
      I feel dumb. Please explain.
      Jan 9
    • Booking.com avQC84
      “Do the needful” & “being kindly” are phrases predominantly/exclusively used by Indians.
      Jan 9
    • Snowflake Computing Bullshitty
      Huh? New information.
      Although I should tell you that no Indian has ever complained about racism. They don't give two shits. Infact, Indians were not offended by Apu on The Simpsons. White people got offended on behalf of Indians.

      In short, Indians couldn't care less about racism against them.
      Jan 9
    • OP
      Do the needful is now part of Blind culture.
      Jan 9
    • Snowflake Computing Bullshitty
      Yes. See? Multi-culturalism and integration works! You just have to have an open mind and not find excuses to use the buzz words.
      Jan 9
    • Vertivco / Eng
      FastPapuan

      VertivcoEng

      PRE
      Youtube, American Bureau of Shipping, Facebook
      FastPapuanmore
      Never heard of these bullshit phrases, and I get hundreds of emails from Indian firms per week.
      Jan 9
    • It's because it's translates differently from Tamil my Southern friend.
      Jan 9
  • Snowflake Computing Bullshitty
    As long as the test suite runs fine, no need to worry. Just give an honest and light suggestion to the person for next time.
    Jan 95
    • OP
      We have no tests for this code.
      Jan 9
    • OP
      I felt so dirty saying that.
      Jan 9
    • Snowflake Computing Bullshitty
      Drop your suggestions on the review and mark unresolved for a few of them. Force him to improve. Also ask for some tests?
      Jan 9
    • OP
      Too late to mark unresolved, but I will go ahead and leave some comments.
      Jan 9
    • Microsoft 4RunnerFTW
      Leave comments regardless and then ping the person directly and say you have some comments. Don’t give a shit about feelings. Bad code is bad code. Open a bug as well it it helps.
      Jan 9
  • Tableau Zero Cool
    Open an issue and add branch policies to be set as a required reviewer the next time
    Jan 90
  • Hubspot bloyg
    There are code inspection tools that will detect duplicated code and classify them as tech debt. The tech debt stats would allow you to justify complaining about the code.
    Jan 91
    • Tableau Zero Cool
      Actually, you should have this static code analysis as a pre-checkin rule.
      Jan 9
  • Northrop Grumman frontbutt
    Most source control tools like github and fecru can have the default changed from 1 sign off to multiple. Also, it may be worth it to enforce a time limit like 5 days or more to give people ample time to review it.

    You should definitely have a team discussion about this so you don't have a new employee approving a pr they don't understand, and the requester merging bad code because "it was approved".

    And uh, if there's no tests for the code, it sounds like you may have more on your plate in the future. (require code changes to be covered, part of the definition of "done", etc... )

    Good luck op
    Jan 90

Join verified employees in our anonymous social network!Download the app!

close