I hate code reviews



  • A mediocre contractor just refactored her code to my suggested slimming down. But she didn't think it through, and now it might turn out the new code has problems the old one didn't.

    Is it too unreasonable to expect people to push back against my code reviews?

    You spent days working on this code. I only look at it for a few minutes.

    Even if I'm senior, you still might be right about your decisions. Think. Challenge me back. You don't have to immediately cave in and change everything to my half-assed suggestion / question.

    Also, I have provided several alternative refactorings. Did you look at every one, or did you just pick the first one?

    And yes, I know this would be more useful as a feedback to the developer, instead of an anonymous rant on a forum. Baby steps.



  • @cartman82 said in I hate code reviews:

    And yes, I know this would be more useful as a feedback to the developer, instead of an anonymous rant on a forum.

    You work in a startup. Knowing startup developers, she might have a nervous breakdown as suddenly she's expected to think for herself instead of doing what she's told.

    And I don't know... it kind of depends on your work relationship. If my peer with similar experience tells me to build an EnterpriseJavaBeanFactoryBuilder to get current date, I'll push back. If it's someone with more seniority, I probably won't - after all, maybe they're right, and even if they're not it's not like I'm going to change their mind.



  • @Maciejasjmj said in I hate code reviews:

    And I don't know... it kind of depends on your work relationship. If my peer with similar experience tells me to build an EnterpriseJavaBeanFactoryBuilder to get current date, I'll push back. If it's someone with more seniority, I probably won't - after all, maybe they're right, and even if they're not it's not like I'm going to change their mind.

    Yeah, I see the problem.

    Junior has all the time in the world, but no experience to make correct decisions. Senior can make correct decisions, but time is too valuable to spend refactoring all the code all the time.

    Since this particular feature was designed by my boss while I was away, it means it was poorly communicated and conceived from the start. So I ended up just shoving it behind a feature flag, until I can refactor it.

    But since we'll probably continue working with this girl, it might pay off to invest more time coaching her.

    Dunno, I hate this human interaction shit. I wish people would just leave me alone to make stuff, instead of having to constantly look at their awful code and babysit them through everything.



  • @cartman82 said in I hate code reviews:

    Is it too unreasonable to expect people to push back against my code reviews?

    You spent days working on this code. I only look at it for a few minutes.

    Even if I'm senior, you still might be right about your decisions. Think. Challenge me back. You don't have to immediately cave in and change everything to my half-assed suggestion / question.

    Meanwhile in some other part of this forum:

    @cartman82 said in Default search target:

    So after all that hemming and hawing, we ended up with the exact same search system I suggested.
    [snipped image, Ed.]

    What we should all take from this is that Cartman is always right. Next time, just accept my suggestions immediately, no need to even think about them.

    Be careful what you wish for...



  • @JBert I happen to be right in both those posts.


  • sockdevs

    @cartman82 said in I hate code reviews:

    @JBert I happen to be right in both those posts.

    of course, when you change your mind in a code review it doesn't mean you were wrong before, it just means you were not in possession of all the facts of the matter, now that you have more facts you of course reach different conclusions.

    ;-)



  • @Maciejasjmj said in I hate code reviews:

    If it's someone with more seniority, I probably won't - after all, maybe they're right, and even if they're not it's not like I'm going to change their mind.

    I've almost always asked for an explanation about why the change at least. As cartman said: he only looked atentamente the Code for a short time


  • Winner of the 2016 Presidential Election

    @Jarry said in I hate code reviews:

    I've almost always asked for an explanation

    I never change anything without a convincing explanation (except code style issues). Anyone who does is missing the point of a code review (discussion and sharing knowledge).



  • @Jarry said in I hate code reviews:

    I've almost always asked for an explanation about why the change atentamente least. As cartman said: he only looked atentamente the Code for a short time

    Your device autocorrects 'at' to that?



  • @coldandtired yeah. It's set to spanish. And It tries hard to make me write spanish



  • @Jarry Make Spanish Great Again?



  • @AlexMedia said in I hate code reviews:

    @Jarry Make Spanish Great Again?

    :tropical_fish:



  • @loopback0 said in I hate code reviews:

    @AlexMedia said in I hate code reviews:

    @Jarry Make Spanish Great Again?

    :tropical_fish:

    The 16th century called and they don't agree with you.



  • @remi said in I hate code reviews:

    The 16th century called and they don't agree with you.

    What's 16th century Spanish for :wambulance: ?



  • @loopback0 said in I hate code reviews:

    What's 16th century Spanish for :wambulance: ?

    el :wambulance:?


  • Dupa

    @cartman82 said in I hate code reviews:

    @Maciejasjmj said in I hate code reviews:

    And I don't know... it kind of depends on your work relationship. If my peer with similar experience tells me to build an EnterpriseJavaBeanFactoryBuilder to get current date, I'll push back. If it's someone with more seniority, I probably won't - after all, maybe they're right, and even if they're not it's not like I'm going to change their mind.

    Yeah, I see the problem.

    Junior has all the time in the world, but no experience to make correct decisions. Senior can make correct decisions, but time is too valuable to spend refactoring all the code all the time.

    Since this particular feature was designed by my boss while I was away, it means it was poorly communicated and conceived from the start. So I ended up just shoving it behind a feature flag, until I can refactor it.

    But since we'll probably continue working with this girl, it might pay off to invest more time coaching her.

    Dunno, I hate this human interaction shit. I wish people would just leave me alone to make stuff, instead of having to constantly look at their awful code and babysit them through everything.

    Sound like you should've taken be a team leader, or at least you shouldn't have junior devs in your team.


  • Winner of the 2016 Presidential Election

    @RaceProUK said in I hate code reviews:

    @loopback0 said in I hate code reviews:

    What's 16th century Spanish for :wambulance: ?

    el la :wambulance:?

    FTFY.


  • Dupa

    @kt_ said in I hate code reviews:

    @cartman82 said in I hate code reviews:

    @Maciejasjmj said in I hate code reviews:

    And I don't know... it kind of depends on your work relationship. If my peer with similar experience tells me to build an EnterpriseJavaBeanFactoryBuilder to get current date, I'll push back. If it's someone with more seniority, I probably won't - after all, maybe they're right, and even if they're not it's not like I'm going to change their mind.

    Yeah, I see the problem.

    Junior has all the time in the world, but no experience to make correct decisions. Senior can make correct decisions, but time is too valuable to spend refactoring all the code all the time.

    Since this particular feature was designed by my boss while I was away, it means it was poorly communicated and conceived from the start. So I ended up just shoving it behind a feature flag, until I can refactor it.

    But since we'll probably continue working with this girl, it might pay off to invest more time coaching her.

    Dunno, I hate this human interaction shit. I wish people would just leave me alone to make stuff, instead of having to constantly look at their awful code and babysit them through everything.

    Sound like you should've taken be a team leader, or at least you shouldn't have junior devs in your team.

    Ok, just a bit of explanation, so I don't come across as an obnoxious shithead. How I view a team leader, or a scrum master or whatever is a kind of a mentor, a person who wants to work with people and who has at least a tad of flair for managing. When you're not willing to do any of this, you probably would be an awesome senior dev, but I'm not sure if you should be a team lead.

    Sure, it could be due my lack of experience when it comes to working with teams consisting mostly of senior devs, cause well… I have yet to work in one structured this way.



  • @cartman82 If you are a contractor you do what you are told.


  • Winner of the 2016 Presidential Election

    @lucas1 said in I hate code reviews:

    @cartman82 If you are a contractor you do what you are told.

    Pretty much the same status as an employee then, which I'm under the impression a lot of legal systems would think of as meaning you are an employee, and the company needs to treat you that way in all ways.



  • @Dreikin It depends. Some orgs treat you as employees.

    Others don't.

    At Capita, Bet365 and a few other places (I can't remember now, I do 2 x 3 months and fuck off abroad for a while) when I worked there you were basically another employee.

    It is totally variable on your employer and where you work in that org.

    Capita in Sheffield was pretty strict, but Capita Southampton I turned up at 11 a few times and no body gave a fuck (I finished at 8 btw).



  • @remi said in I hate code reviews:

    The 16th century called and they don't agree with you.

    1588 does.



  • @cartman82 said in I hate code reviews:

    Is it too unreasonable to expect people to push back against my code reviews?

    No. Everyone here (small shop, but still) will argue against a revision if they feel there's a good reason for it. Everyone is generally reasonable, so sometimes things get changed and sometimes they're left alone. We also do peer reviews so I've been on both sides.

    That doesn't necessarily solve your first problem. Even after discussion change might be required, but that doesn't fix an incorrect change. In that case you should work though issues together so she understands what she missed.



  • @fwd I think code review should be done via mandatory beating,



  • @cartman82 said in I hate code reviews:

    Dunno, I hate this human interaction shit. I wish people would just leave me alone to make stuff, instead of having to constantly look at their awful code and babysit them through everything.

    That's a problem, you see, as most Real World Programming Jobs involve 10% coding, debugging and other technical things and 80% human interaction shit (the other 10% is surfing porn when you think no one is watching).



  • @ScholRLEA said in I hate code reviews:

    (the other 10% is surfing porn when no one is watching)..

    Huh, I must be doing it wrong,



  • @JBert I would say so! You need the practice, in case you are ever promoted to middle management and have to spent 30% of the day on it. Get fapping!



  • @JBert said in I hate code reviews:

    @ScholRLEA said in I hate code reviews:

    (the other 10% is surfing porn when no one is watching)..

    Huh, I must be doing it wrong,

    Only when someone is watching?



  • @lucas1 said in I hate code reviews:

    @cartman82 If you are a contractor you do what you are told.

    Yeah, but I don't want to tell you.

    I don't want a voice controlled automaton who types code I tell them. I don't have time to handhold you.

    I need a person who cares about what they are making and can take initiative and do the right thing without being coaxed for every little thing. I want colleagues whose branches I can just merge without having to go through every little thing and hunt for bugs they missed and lawyer with them over whether their screwups are technically bugs or just crappy code.

    Maybe I am too demanding, I don't know. But I am frankly getting sick of working with IMO mediocre people.



  • @kt_ said in I hate code reviews:

    Sound like you should've taken be a team leader, or at least you shouldn't have junior devs in your team

    I am the team lead.

    And i dont have a choice about the people boss draggs in



  • @kt_ said in I hate code reviews:

    Ok, just a bit of explanation, so I don't come across as an obnoxious shithead. How I view a team leader, or a scrum master or whatever is a kind of a mentor, a person who wants to work with people and who has at least a tad of flair for managing. When you're not willing to do any of this, you probably would be an awesome senior dev, but I'm not sure if you should be a team lead.

    Yeah fair enough. I want to code with equals and betters, not coddle unmotivated shitheads. This girl might not turn out bad, but since she's contracting, theres zero point investing my time in mentoring her.


  • Discourse touched me in a no-no place

    @cartman82 said in I hate code reviews:

    This girl might not turn out bad, but since she's contracting, theres zero point investing my time in mentoring her.

    Depends on the length of the contract. A short-term hire? You're absolutely right.



  • @dkf shes not even coming to the office. Working after hours a bit, but mostly while pretending to work on her dayjob.

    All these people boss finds are like that. No wonder they are mediocre.

    Never marry a cheater. You'll just get cheated.


Log in to reply
 

Looks like your connection to What the Daily WTF? was lost, please wait while we try to reconnect.