51–60 of 81 posts
More than anything else, though, I'm curious where teams like this still go to buy those amazing Apple Cinema displays?!
Earlier quoted context omitted.
Noobie question..... Do you think there is any value in writing the code with the correct style before it is fixed in the compiler? Like is it easier to help out and spot a problem quicker, if everyone is thinking in the exact same patterns when they write code?
Depends on the kind of style 'subject' I think. If it is trivial indentation, bracket placement and other preference kind of things it probably wouldn't. But if the style things lean more towards design patterns, then often there is a proper reason behind choosing that style over another and it would help because you would notice doing it the 'wrong' way and you stop to evaluate and maybe spot a bug or design issue.…
Styles like forbidding single-statement if conditions that don't have curly braces helps protect you from bugs like goto-fail[0] meanwhile styles like which line your opening curly goes on, use of spacing, dare I say tabs vs spaces (outside of languages with significant whitespace anyway), and even abbrev names vs BigLongNames[1] don't really matter apart from personal preference and it being aesthetically pleasing to have local consistency.
The real problem here are people with bad attitude and poor communication skills. Bullying other people is a punishable offense in real life, and I don't see why code reviews should be any different. Have some basic rules regarding communication. Issue a public warning for first-time offenders. Give them the boot if they do it again. There, problem solved.
Earlier quoted context omitted.
If someone is that much more experienced than you are, it can be hard for you to tell the difference between nitpicking and something that experience has taught them is more important than you realize. Even if it is just nitpicking, that still means your code has nits to pick.
Code reviews are generally public and visible to the entire team and often even to other teams. While it's true that the reviewee may not be able to distinguish the two cases being considered, there may be other developers on the team or in the company who witness the review and can make the distinction. In fact, this can cause the "toxic reviewing" practice to spread to other experienced senior developers, who may s…
Most code reviews I've participated in have devolved into the senior/loudest person making sure that people know who they are and what everyone's place in the hierarchy is. Even in reviews where the reviewee had some amazing code that was kind of groundbreaking in one area, still have to find something to gripe about because I have 20 years on this kid. This seems to be quite common in science/engineering, not just i…
- bugs found in code reviews MUST be fixed
- coding style violations MUST be fixed (ideally all of these are caught by lint but not all are)
- everything else is treated as optional
Some code review tools even provide the ability to specify required vs. optional feedback. We now use github reviews which doesn't have this feature so we usually just add OPTIONAL to any review comment that isn't in the MUST fix category.
Occasionally a reviewer and reviewee will get into discussion/disagreement over the approach a particular code change. In those cases if it isn't clearly a bug or a style violation we err on the side of the reviewee.
Most code reviews I've participated in have devolved into the senior/loudest person making sure that people know who they are and what everyone's place in the hierarchy is. Even in reviews where the reviewee had some amazing code that was kind of groundbreaking in one area, still have to find something to gripe about because I have 20 years on this kid. This seems to be quite common in science/engineering, not just i…
Not sure why this is being down voted. This is exactly the number one thing I have experienced happening with code reviews, especially peer reviews. This attitude often completely breaks the effectiveness of the whole process, making it a deal breaker for quick iterations and agile projects.
Earlier quoted context omitted.
Code reviews are generally public and visible to the entire team and often even to other teams. While it's true that the reviewee may not be able to distinguish the two cases being considered, there may be other developers on the team or in the company who witness the review and can make the distinction. In fact, this can cause the "toxic reviewing" practice to spread to other experienced senior developers, who may s…
Interesting - I thought you would end the argument differently. In my experience additional visibility helps curb the unwanted behaviour because other senior devs step in and help fight it - but I guess that depends on the general culture in the company. If that doesn't happen you have bigger problems anyway.
And there's definitely a circularity here. In a healthy culture, code reviews tend not to be overly political in the first place, so it would stand out like a sore thumb if someone was causing a lot of unnecessary drama with their reviews. Likewise, in a toxic culture, code reviews may go from healthy to toxic and dramatic because developers have been incentivized to compete with one another in counterproductive ways.
Most code reviews I've participated in have devolved into the senior/loudest person making sure that people know who they are and what everyone's place in the hierarchy is. Even in reviews where the reviewee had some amazing code that was kind of groundbreaking in one area, still have to find something to gripe about because I have 20 years on this kid. This seems to be quite common in science/engineering, not just i…
> still have to find something to gripe about because I have 20 years on this kid I wish I had someone with decades more experience than me taking the time to find areas where my code could improve. Regardless of their motivation, this sounds like an amazing resource that you're lucky to have.
Let's say you send code out for review that is just about perfect how it is. The reviewer is faced with a choice. If they don't suggest any changes, they may feel it gives the impression that you are just good a coder as they are, that their input isn't needed, or that they didn't bother to read it and just rubber stamped it. All of these challenge their place in the organization as a technical thought leader and influencer.
Some people have the maturity and awareness to just say, "Great job. I see no reason to change any of this. Approved." Some people don't, so they will dig for something negative to say. Even if it is just "these variable names are excessively long, and it's distracting to read" or "you don't have enough parameters to warrant the use of the Builder pattern here, so take it out".
Earlier quoted context omitted.
if a = foo() versus: if (a = foo()) Sometimes idiomatic patterns vary based on things which are not amiable to mechanical correction.
In what language do those produce different results?
Earlier quoted context omitted.
So "I haven't experienced this before therefore it doesn't exist. Have a downvote :)"? I kind of agree that the parent poster's tone could have been chosen with a bit more care though.
OP is implying that this problem exists everywhere. This is simply not the case.
But oh look, it's no longer being downvoted. Perhaps there's something to this after all?