6 ms·

We are ruthless on code reviews

51 points by grey 10y ago

gjolund 10y ago
I love working in environments like this. I can say hands down they were my most productive years as a programmer. Quality code review is the single most important trait of a high performing senior programmer.
- computmaxer 10y ago
  As someone who works at Workiva, I completely agree. The post comes off a little more abrasive than I'd like. I think writing it as an "email" to a hypothetical new team member makes it worse. The point is: we take our code reviews seriously and we may point out things that seem silly or nitpicky, we may question your approach, we may make many suggestions for improvement, but at the end of the day it is nothing personal. It is for the common goal of high code quality.
  - gjolund 10y ago
    Code review is typically how I structure my interviews. I try not to ask arbitrary comp-sci questions, but instead provide the candidate with a block of code and ask them to provide a comprehensive code review (usually through a github pr). Afterwards I perform a my review, and highlight any differences in style or methodologies. I'm not really looking for people that have the exact same outlook as me, I'm just looking for people that have some kind of standard they adhere to.
- bobbytherobot 10y ago
  Before people take and run with this comment, this will turn into a shit show if you don't have actual senior programmers with the needed experience. I've seen it happen in startups where their "senior programmer" has only been working for three years out of school for the single startup.
  - falcolas 10y ago
    This is absolutely the truth, and based on my experiences, absolutely a problem. New graduates are promoted through the ranks at startups so quickly, yet they have terrible interpersonal skills, and their code reeks of naivety. I've watched an "architect" from such a company write the most tangled spaghetti western of code for the simplest CRUD app that even his lower level colleagues were appalled. Of course, those same colleagues then advocated a complete re-write of the working codebase instead of incrementally fixing the code, causing about 6 months of new work for the company before they could start on hardening or new features... No, experience doesn't automatically make you right, but it does increase the likelihood.
  - gjolund 10y ago
    While I understand where you are coming from, I don't see how your comment adds anything to the conversation. If you don't have senior staff then you are going to have a host of other problems as well.
- cpjk 10y ago
  My first internship was like this. I loved it. It provided me with instant feedback on my code and as a result the quality of my code went from probably pretty low to as good as I could make it. Having now worked at other places where code review is given low priority, I can say with confidence that I don't want to work anywhere that does not do serious code review. Code review provides an important mentorship opportunity for junior programmers, and provides a simple way to keep code quality high and consistent. In addition, it provides a simple way to instruct new hires on internal style and conventions.
draw_down 10y ago
If it's anything like where I work, some people come in for more ruthlessness than others. Some are more equal than others, if you will. New people or those deemed unworthy will receive the full brunt of scrutiny, to the point of bare-faced pedantry. Those who are golden are given the benefit of the doubt. You can probably tell which one I am. It gets really old to see this time and again, and to see the team pat themselves on the back for being "ruthless".
- noamsml 10y ago
  Playing devil's advocate here: The only way to really learn the full ruleset of a team is by undergoing a couple of code reviews in which every issue is pointed out. Once your teammates see you've understood the prevailing style, they don't have to work as hard at code reviews. That said, I usually hate petty/nitpicky code reviews. I try to focus only on issues that I think affect readability, maintainability or functionality.
  - Niksko 10y ago
    Makes sense, but there are ways of being even petty and nitpicky without being condescending or rude. State facts, don't throw insults, don't demean or belittle.
    - coredog64 10y ago
      One good rule I was given by a tech lead is to not use the word "you" in the PR comments. PR comments are a discussion on the failure of the code, not of the individual. Avoiding "you" allows for some separation between the two.
      - cachemiss 10y ago
        I always use "we", and I almost always phrase things as a question. Example: "So we are doing X here, which I think will probably do Y, which could have adverse affects Z, are we sure we want to do this?"
  - 10y ago
MisterBastahrd 10y ago
If you've got time to dream up ways to be witty in order to be snarky to a co-worker, then you've got too much time on your hands at work and clearly need to fill it with other responsibilities.
- savanaly 10y ago
  I would hazard a guess that this letter was to a hypothetical employee, rather than for a real situation. And its purpose was to illustrate the type of code review new employees can expect and are expected to give, in a whimsical way. I quite liked it.
- yolesaber 10y ago
  That's right! Get back to work. That's all you're good for, after all. Just keep working! Have a funny thought you want to jot down? Sorry pal forget it you clearly should be working, endlessly, tirelessly. Don't forget you're here forever. Working.
joesmo 10y ago
Yet despite being assholes (ruthless in their parlance) who obviously have not on-boarded said employee or even told him what is expected of his code, I doubt their software has any less bugs than the software from shops who are not assholes (ruthless in their parlance). Sounds like a great reason to avoid working here and other places that have the same mentality. If you want your coworkers to actually do their work, it's a really good idea not to be an asshole because it's incredibly easy to get away with not doing shit in tech while seeming to do so and after awhile, most people will switch to that mode when berated long enough. I can't blame them. Employers deserve no loyalty in the US these days because they don't give any.
- hvidgaard 10y ago
  In this articles terms, I'm ruthless when doing code reviews. I do it because * We have a code style to follow, and if we don't we might as well not have it. * It ups the game from the developers, they double check their code before checkin, and believe it or not, they catch more errors this way. * It have created an environment where we can openly talk about interative improvements.
parennoob 10y ago
The whole "we are so cool because we are ruthless. Merciless, I tell you! Once you get through our code review your code will be perfect." attitude of this article is a bit surprising for one ranked so highly on HN. Do people really prefer this kind of attitude? Bender GIFs and snarky comments can become quickly tiring. I personally would strongly prefer if they put it in milder technical terms, "We are very conservative with deviations from our preferred code style and conventions". I suppose that wouldn't be as click-baity, but for me (and I suspect a lot of other people) it would be easier to appreciate, since we understand that being conservative before accepting commits leads to a well-groomed codebase.
- savanaly 10y ago
  Every single time I do a code review I find myself noticing a lot of little style mistakes (and, not to be overconfident, I could be mistaken myself about that) and don't leave a comment because I don't want to be that guy that no one wants to review their code and gives one a major headache because I care more about the consistency than does the person I'm reviewing. In other words, I might restrain myself from criticism out of my compassion for my coworkers. The definition of "ruthless" according to Google is "having or showing no pity or compassion for others," so it's a fine term to describe how I'm not-- and how this company is determined to be.
  - parennoob 10y ago
    Yeah I think this kind of thing is a teamwide preference. If you prefer very rigid styling and enforce a style checker, great – that can be put into a script which you compulsorily run before review.
  - mkozlows 10y ago
    As a team, decide how much you care. Some teams won't care much at all, and will follow a "hey as long as it works" attitude. Other teams will be deeply interested in making their code as good as it could be, and will want comments about doing things more cleanly or elegantly and will view PR review as a chance to learn and improve. As long as everyone's on the same page, and sharing the same attitude, that drive to improve will come across as a positive thing rather than a jerk move. But if you're the one person with that attitude on a team of good-enoughers, yeah, not so much.
estefan 10y ago
People find it acceptable to write more harshly than they'd speak face-to-face. I wonder if all 43 or whatever points would have come up if it was done side-by-side the developer. Also, great negative PR for this company. This sort of discussion would also come off as much less arrogant/snarky face-to-face.
eddieroger 10y ago
I guess that's one way of doing it, but you're dependent on the quality of said ruthless reviewers, and delivery of the message is separate from the message itself. Personally, I'm more of a "kill them with kindness" guy, so while my reviews would come off as soft by the article's definition, I'd argue that they're just as effective, and my teammates still want to have a beer with me at the end of the day, instead of walking away thinking I have a stick up my ass, which an email about ruthlessness wouldn't necessarily negate.
siegecraft 10y ago
God, how insufferable. "Our code reviews are so ruthless because our clients (and by extension, us) are so much better than everyone else!" But apparently we can't write a bot or a pre-commit hook to re-order imports so instead we'll write some non-sensical in-house jargon (because typing out the full explanation is too much effort, even though we are the best of the best because our clients are so excellent and demanding). But don't worry, we'll write a 2 page email to explain this jargon.
scolfax 10y ago
- If this post is satire, it's hilarious and spot-on! - If this is real, quit while you can, and may god have mercy on your soul. - The fact that it's difficult to tell if it's real or not says a lot about our industry.
- cpjk 10y ago
  It's not satire, it's hypothetical.
  - goldenkey 10y ago
    It reeks of humble bragging and pretentiousness. The Dark Souls analogy just tops it off...
    - cpjk 10y ago
      Reading it again, I can see that. However, I personally am a fan of rigorous - not ruthless - code review, the purpose and level of which has previously been explained to a new employee. Obviously being an asshole is out of line, and if someone is being a dick in code review, that's a problem with the reviewer. But I think that being rigorous about maintaining consistency and quality is important.
      - goldenkey 10y ago
        Decent positions dont usually need disclaimers. Theres a difference between preaching, teaching, and coaching. Its not obvious that being an asshole is out of line. You make it sound like this mystery of pure "collaboration" is all about giving raw feedback. Raw feedback is inferior to polished feedback. Its just as important to communicate in positive ways instead of throwing this ruthless word around. Honestly, your problem is that youve approached code discussion from a purely quantitative approach - ability to convince and ability to convey arguments in a persuasive yet unbiased way is huge. No one wants to do things by demand, decree, or just general hate.
        10y ago
jorjordandan 10y ago
This reminds me of people who say, "People don't like me because I'm so honest." No, people don't like you because you lack basic empathy.
- mfringel 10y ago
  Amen. Bonus points if they try to dress it up with some honorific like "Truthful Speaker".
- themartorana 10y ago
  s/lack basic empathy/are an asshole/
nmeofthestate 10y ago
Code reviews are pretty heavy going on the project I'm working on at the moment. The code isn't noticeably better for it though!
- ybobinator 10y ago
  You gotta argue your side if what they're saying is going to make shit awful. IMO the best way to do it is to try their approach out and show them in a few lines that you don't think it's helping. Doing whatever people say just because is bad and so is just saying no. You gotta work with them.
  - nmeofthestate 10y ago
    Don't worry - there's been plenty of long fruitless debates along the way.
kabdib 10y ago
Satire, yes? If not, GTFO while you can.
mahyarm 10y ago
If you have things in your style guide that can be done easily by a script, they shouldn't ever be interm rules. They should be a linter auto-fix rule that applies it to diffs automatically and NOTHING ELSE. It shouldn't even blip at you saying it's against style, it should just fix it. If you don't have this, you waste everyone's brain cycles with your pedantic import ordering rule or similar on both sides and it's a red flag about the company to me.
aelaguiz 10y ago
Based on this I can only assume that workiva builds spaceships or life support systems. You know, something where absolute perfection is far more important than having a fulfilling and constructive workplace environment.
auggierose 10y ago
It always astonished me how delicate coders in the UK are (I am from Germany, working in the UK currently). During a team meeting at a former company, my "ruthlessness" was brought up as a negativism. I countered that (or so I thought) with the following question to the rest of the coder team: "Do you want me to point out problems I perceive with the code / approach / whatever, or do you want me to hold back in order to be considerate of someone". To my surprise, in the presence of management, the unanimous answer was to show consideration! I think people who work together should be able to openly communicate with each other about technical issues. Sadly, most people are (either self-perceived or for real) not good enough to do that without bruised egos.
- brandmeyer 10y ago
  There is another side to this. I've seen a ton of technical criticism get delivered with emotionally laden commentary. X is "totally broken", "crap", "not even wrong", "just wrong", "terrible", etc. There is a way to deliver criticism while maintaining an environment of common professional respect. Many people don't even try. With the caveat that I'm painting with a culturally biased brush, I've found Germans (and Finns!) to be particularly abrasive in this regards.
  - cpks 10y ago
    And conversely, I've seen people from cultures with different communications styles substantially and unfairly discriminated in the industry for not matching 2016 American business culture. I've found Germans (and Finns!) to be particularly discriminated in this regard, primarily because those are highly technical cultures, but with different communication styles. However, anyone who grew up in the inner city, in African American culture, in Irish Catholic culture, etc. is likewise stigmatized and labeled as rude due to what is a cultural communications difference.
- skellington 10y ago
  It's interesting that even your comment is getting down voted. I wonder if it's a millennial thing? It would be interesting to know the ages of the people here commenting on how "harsh" the post seems vs. "yeah that's great." I read the post as going out of it's way to give praise while pointing out what the company requires (perhaps that is interpreted as patronizing). But many people here are commenting on how rude it is. Bizarre. If you think this is rude, you've never worked in a "tough" environment. How would people here suggest communicating the same information? Is it just that it might be interpreted as patronizing that's the problem?
crdoconnor 10y ago
>I don’t mean we’re mean-spirited. I just mean that we are merciless. You’ll notice that I left the comment “Beep!” on the imports of every file you touched. What I meant was, “Your imports violate our standard convention—we order them by built-ins, then third party, and then project level,” but that was too much to type on every file. Translation: "We have extremely strong opinions on what materials to use for our nuclear power plant's bike shed." https://en.wikipedia.org/wiki/Law_of_triviality https://en.wikipedia.org/wiki/Law_of_triviality There's seems to be an inverse relationship between how much effort a team spends on surface details of code (e.g. does it adhere to linting standards/are the lines < 80 characters/imports ordered?) and how much time and effort a team spends on deeper architectural issues (loose coupling, de-duplication, un-reinventing the wheel, transforming imperative to declarative code, etc.)
- Ensorceled 10y ago
  Until the first time you spend a couple of hours looking for a weird bug in Python because somebody broke this rule...
  - crdoconnor 10y ago
    Which rule? The rule that imports must be ordered according to the whims of the team lead? Or the rule that lines must be under 80 characters? There are a few linting rules which it pays major dividends to pay attention to (e.g. variables that are initialized but never used), but most of them are superficial.
    - Ensorceled 10y ago
      You commented on the import ordering so clearly that's what I'm commenting on. Not following the PEP standard for python import ordering can cause issues like circular imports or monkey patching not working properly.
jbrooksuk 10y ago
As a PHP "shop" this is why we use StyleCI [1]. Our code isn't that critical, the quality and maintainability are. We don't waste time on "code reviews" that only check the style of code, that can be automated away. Yes, I make StyleCI, but we use it in the place I work too (unrelated). [1] https://styleci.io https://styleci.io
dlandis 10y ago
Ugh, comes off as condescending, immature, and insufferable. I do think strict code review and high standards are the way to go, but when implementing them it becomes all the more important to have appropriate and professional interpersonal communication and documented policies.
louden 10y ago
'We are rigorous on code reviews' would be a better approach. We don't have to be jerks to make sure that code is up to standards.
jkot 10y ago
For me declaration of "ruthlessness" is a red flag, similar to ninja buzzword. Strictness in one direction does not mean company is strict at all fronts. Code reviews can be used without automated tests or version control. In this case the review process seems like a "busy work". Checks bellow could be easily integrated into build script or CI. Such code should not even made it into code review. (I am not arguing about the rule itself, just how it should be enforced) > You’ll notice that I left the comment “Beep!” on the imports of every file you touched. What I meant was, “Your imports violate our standard convention—we order them by built-ins, then third party, and then project level,” but that was too much to type on every file.
gdulli 10y ago
It's probably a joke but it's backfiring because most comments are taking it seriously and thinking the place actually works like this.
lcfcjs 10y ago
The code review process at my last job completely ripped the company apart. It has to be handled with a ton of consideration. Especially, if you are trying to communicate in comments on a Pull Request (and the developer is remote). It needs to be a learning experience, rather than a bollocking of a lesser individual. There is a good way to suggest changes, and a bad way. Developers are so often terrible at communication.
kvcrawford 10y ago
I miss working in an environment like this. I was actually surprised to find so much negative feedback in the comments. I think a detail getting glossed over is that the tone needs to be calibrated to the relationships between team members. If team members joke around with each other, send silly gifs, get lunch or coffee together, and are generally on good terms with one another...then it's known that you don't take PR comments personally, they're just feedback. And the more critical the feedback, the better developer you become.
- Dommorak 10y ago
  I can tell you that this is the kind of environment I work in and the first thing I teach a new teammate is that the team is only concerned with consistent high quality code. I think it is a good lesson for most developers and software engineers to separate their personal feelings from their code. It can help you be less defensive of your approach and take in multiple viewpoints to find the best possible solution for the task at hand.
cafard 10y ago
"One thing to keep in mind is that our customer base is unique. Our customers work for many of the most successful companies in their respective industries. They are where they are because they are committed to excellence and have great attention to detail." An awful lot of companies can describe their customers in such terms. Microsoft can, Google can, Pepsi and Bic can.

51 points by grey 10y ago

I love working in environments like this. I can say hands down they were my most productive years as a programmer. Quality code review is the single most important trait of a high performing senior programmer.

computmaxer 10y ago
As someone who works at Workiva, I completely agree. The post comes off a little more abrasive than I'd like. I think writing it as an "email" to a hypothetical new team member makes it worse. The point is: we take our code reviews seriously and we may point out things that seem silly or nitpicky, we may question your approach, we may make many suggestions for improvement, but at the end of the day it is nothing personal. It is for the common goal of high code quality.
- gjolund 10y ago
  Code review is typically how I structure my interviews. I try not to ask arbitrary comp-sci questions, but instead provide the candidate with a block of code and ask them to provide a comprehensive code review (usually through a github pr). Afterwards I perform a my review, and highlight any differences in style or methodologies. I'm not really looking for people that have the exact same outlook as me, I'm just looking for people that have some kind of standard they adhere to.
bobbytherobot 10y ago
Before people take and run with this comment, this will turn into a shit show if you don't have actual senior programmers with the needed experience. I've seen it happen in startups where their "senior programmer" has only been working for three years out of school for the single startup.
- falcolas 10y ago
  This is absolutely the truth, and based on my experiences, absolutely a problem. New graduates are promoted through the ranks at startups so quickly, yet they have terrible interpersonal skills, and their code reeks of naivety. I've watched an "architect" from such a company write the most tangled spaghetti western of code for the simplest CRUD app that even his lower level colleagues were appalled. Of course, those same colleagues then advocated a complete re-write of the working codebase instead of incrementally fixing the code, causing about 6 months of new work for the company before they could start on hardening or new features... No, experience doesn't automatically make you right, but it does increase the likelihood.
- gjolund 10y ago
  While I understand where you are coming from, I don't see how your comment adds anything to the conversation. If you don't have senior staff then you are going to have a host of other problems as well.
cpjk 10y ago
My first internship was like this. I loved it. It provided me with instant feedback on my code and as a result the quality of my code went from probably pretty low to as good as I could make it. Having now worked at other places where code review is given low priority, I can say with confidence that I don't want to work anywhere that does not do serious code review. Code review provides an important mentorship opportunity for junior programmers, and provides a simple way to keep code quality high and consistent. In addition, it provides a simple way to instruct new hires on internal style and conventions.

draw_down 10y ago

If it's anything like where I work, some people come in for more ruthlessness than others. Some are more equal than others, if you will. New people or those deemed unworthy will receive the full brunt of scrutiny, to the point of bare-faced pedantry. Those who are golden are given the benefit of the doubt. You can probably tell which one I am. It gets really old to see this time and again, and to see the team pat themselves on the back for being "ruthless".

noamsml 10y ago
Playing devil's advocate here: The only way to really learn the full ruleset of a team is by undergoing a couple of code reviews in which every issue is pointed out. Once your teammates see you've understood the prevailing style, they don't have to work as hard at code reviews. That said, I usually hate petty/nitpicky code reviews. I try to focus only on issues that I think affect readability, maintainability or functionality.
- Niksko 10y ago
  Makes sense, but there are ways of being even petty and nitpicky without being condescending or rude. State facts, don't throw insults, don't demean or belittle.
  - coredog64 10y ago
    One good rule I was given by a tech lead is to not use the word "you" in the PR comments. PR comments are a discussion on the failure of the code, not of the individual. Avoiding "you" allows for some separation between the two.
    - cachemiss 10y ago
      I always use "we", and I almost always phrase things as a question. Example: "So we are doing X here, which I think will probably do Y, which could have adverse affects Z, are we sure we want to do this?"
- 10y ago

MisterBastahrd 10y ago

If you've got time to dream up ways to be witty in order to be snarky to a co-worker, then you've got too much time on your hands at work and clearly need to fill it with other responsibilities.

savanaly 10y ago
I would hazard a guess that this letter was to a hypothetical employee, rather than for a real situation. And its purpose was to illustrate the type of code review new employees can expect and are expected to give, in a whimsical way. I quite liked it.
yolesaber 10y ago
That's right! Get back to work. That's all you're good for, after all. Just keep working! Have a funny thought you want to jot down? Sorry pal forget it you clearly should be working, endlessly, tirelessly. Don't forget you're here forever. Working.

joesmo 10y ago

Yet despite being assholes (ruthless in their parlance) who obviously have not on-boarded said employee or even told him what is expected of his code, I doubt their software has any less bugs than the software from shops who are not assholes (ruthless in their parlance). Sounds like a great reason to avoid working here and other places that have the same mentality. If you want your coworkers to actually do their work, it's a really good idea not to be an asshole because it's incredibly easy to get away with not doing shit in tech while seeming to do so and after awhile, most people will switch to that mode when berated long enough. I can't blame them. Employers deserve no loyalty in the US these days because they don't give any.

hvidgaard 10y ago
In this articles terms, I'm ruthless when doing code reviews. I do it because * We have a code style to follow, and if we don't we might as well not have it. * It ups the game from the developers, they double check their code before checkin, and believe it or not, they catch more errors this way. * It have created an environment where we can openly talk about interative improvements.

parennoob 10y ago

The whole "we are so cool because we are ruthless. Merciless, I tell you! Once you get through our code review your code will be perfect." attitude of this article is a bit surprising for one ranked so highly on HN. Do people really prefer this kind of attitude? Bender GIFs and snarky comments can become quickly tiring. I personally would strongly prefer if they put it in milder technical terms, "We are very conservative with deviations from our preferred code style and conventions". I suppose that wouldn't be as click-baity, but for me (and I suspect a lot of other people) it would be easier to appreciate, since we understand that being conservative before accepting commits leads to a well-groomed codebase.

savanaly 10y ago
Every single time I do a code review I find myself noticing a lot of little style mistakes (and, not to be overconfident, I could be mistaken myself about that) and don't leave a comment because I don't want to be that guy that no one wants to review their code and gives one a major headache because I care more about the consistency than does the person I'm reviewing. In other words, I might restrain myself from criticism out of my compassion for my coworkers. The definition of "ruthless" according to Google is "having or showing no pity or compassion for others," so it's a fine term to describe how I'm not-- and how this company is determined to be.
- parennoob 10y ago
  Yeah I think this kind of thing is a teamwide preference. If you prefer very rigid styling and enforce a style checker, great – that can be put into a script which you compulsorily run before review.
- mkozlows 10y ago
  As a team, decide how much you care. Some teams won't care much at all, and will follow a "hey as long as it works" attitude. Other teams will be deeply interested in making their code as good as it could be, and will want comments about doing things more cleanly or elegantly and will view PR review as a chance to learn and improve. As long as everyone's on the same page, and sharing the same attitude, that drive to improve will come across as a positive thing rather than a jerk move. But if you're the one person with that attitude on a team of good-enoughers, yeah, not so much.

estefan 10y ago

People find it acceptable to write more harshly than they'd speak face-to-face. I wonder if all 43 or whatever points would have come up if it was done side-by-side the developer. Also, great negative PR for this company. This sort of discussion would also come off as much less arrogant/snarky face-to-face.

eddieroger 10y ago

I guess that's one way of doing it, but you're dependent on the quality of said ruthless reviewers, and delivery of the message is separate from the message itself. Personally, I'm more of a "kill them with kindness" guy, so while my reviews would come off as soft by the article's definition, I'd argue that they're just as effective, and my teammates still want to have a beer with me at the end of the day, instead of walking away thinking I have a stick up my ass, which an email about ruthlessness wouldn't necessarily negate.

siegecraft 10y ago

God, how insufferable. "Our code reviews are so ruthless because our clients (and by extension, us) are so much better than everyone else!" But apparently we can't write a bot or a pre-commit hook to re-order imports so instead we'll write some non-sensical in-house jargon (because typing out the full explanation is too much effort, even though we are the best of the best because our clients are so excellent and demanding). But don't worry, we'll write a 2 page email to explain this jargon.

scolfax 10y ago

- If this post is satire, it's hilarious and spot-on! - If this is real, quit while you can, and may god have mercy on your soul. - The fact that it's difficult to tell if it's real or not says a lot about our industry.

cpjk 10y ago
It's not satire, it's hypothetical.
- goldenkey 10y ago
  It reeks of humble bragging and pretentiousness. The Dark Souls analogy just tops it off...
  - cpjk 10y ago
    Reading it again, I can see that. However, I personally am a fan of rigorous - not ruthless - code review, the purpose and level of which has previously been explained to a new employee. Obviously being an asshole is out of line, and if someone is being a dick in code review, that's a problem with the reviewer. But I think that being rigorous about maintaining consistency and quality is important.
    - goldenkey 10y ago
      Decent positions dont usually need disclaimers. Theres a difference between preaching, teaching, and coaching. Its not obvious that being an asshole is out of line. You make it sound like this mystery of pure "collaboration" is all about giving raw feedback. Raw feedback is inferior to polished feedback. Its just as important to communicate in positive ways instead of throwing this ruthless word around. Honestly, your problem is that youve approached code discussion from a purely quantitative approach - ability to convince and ability to convey arguments in a persuasive yet unbiased way is huge. No one wants to do things by demand, decree, or just general hate.
      - 10y ago

jorjordandan 10y ago

This reminds me of people who say, "People don't like me because I'm so honest." No, people don't like you because you lack basic empathy.

mfringel 10y ago
Amen. Bonus points if they try to dress it up with some honorific like "Truthful Speaker".
themartorana 10y ago
s/lack basic empathy/are an asshole/

nmeofthestate 10y ago

Code reviews are pretty heavy going on the project I'm working on at the moment. The code isn't noticeably better for it though!

ybobinator 10y ago
You gotta argue your side if what they're saying is going to make shit awful. IMO the best way to do it is to try their approach out and show them in a few lines that you don't think it's helping. Doing whatever people say just because is bad and so is just saying no. You gotta work with them.
- nmeofthestate 10y ago
  Don't worry - there's been plenty of long fruitless debates along the way.

kabdib 10y ago

Satire, yes? If not, GTFO while you can.

mahyarm 10y ago

If you have things in your style guide that can be done easily by a script, they shouldn't ever be interm rules. They should be a linter auto-fix rule that applies it to diffs automatically and NOTHING ELSE. It shouldn't even blip at you saying it's against style, it should just fix it. If you don't have this, you waste everyone's brain cycles with your pedantic import ordering rule or similar on both sides and it's a red flag about the company to me.

aelaguiz 10y ago

Based on this I can only assume that workiva builds spaceships or life support systems. You know, something where absolute perfection is far more important than having a fulfilling and constructive workplace environment.

auggierose 10y ago

It always astonished me how delicate coders in the UK are (I am from Germany, working in the UK currently). During a team meeting at a former company, my "ruthlessness" was brought up as a negativism. I countered that (or so I thought) with the following question to the rest of the coder team: "Do you want me to point out problems I perceive with the code / approach / whatever, or do you want me to hold back in order to be considerate of someone". To my surprise, in the presence of management, the unanimous answer was to show consideration! I think people who work together should be able to openly communicate with each other about technical issues. Sadly, most people are (either self-perceived or for real) not good enough to do that without bruised egos.

brandmeyer 10y ago
There is another side to this. I've seen a ton of technical criticism get delivered with emotionally laden commentary. X is "totally broken", "crap", "not even wrong", "just wrong", "terrible", etc. There is a way to deliver criticism while maintaining an environment of common professional respect. Many people don't even try. With the caveat that I'm painting with a culturally biased brush, I've found Germans (and Finns!) to be particularly abrasive in this regards.
- cpks 10y ago
  And conversely, I've seen people from cultures with different communications styles substantially and unfairly discriminated in the industry for not matching 2016 American business culture. I've found Germans (and Finns!) to be particularly discriminated in this regard, primarily because those are highly technical cultures, but with different communication styles. However, anyone who grew up in the inner city, in African American culture, in Irish Catholic culture, etc. is likewise stigmatized and labeled as rude due to what is a cultural communications difference.
skellington 10y ago
It's interesting that even your comment is getting down voted. I wonder if it's a millennial thing? It would be interesting to know the ages of the people here commenting on how "harsh" the post seems vs. "yeah that's great." I read the post as going out of it's way to give praise while pointing out what the company requires (perhaps that is interpreted as patronizing). But many people here are commenting on how rude it is. Bizarre. If you think this is rude, you've never worked in a "tough" environment. How would people here suggest communicating the same information? Is it just that it might be interpreted as patronizing that's the problem?

crdoconnor 10y ago

>I don’t mean we’re mean-spirited. I just mean that we are merciless. You’ll notice that I left the comment “Beep!” on the imports of every file you touched. What I meant was, “Your imports violate our standard convention—we order them by built-ins, then third party, and then project level,” but that was too much to type on every file. Translation: "We have extremely strong opinions on what materials to use for our nuclear power plant's bike shed." https://en.wikipedia.org/wiki/Law_of_triviality https://en.wikipedia.org/wiki/Law_of_triviality There's seems to be an inverse relationship between how much effort a team spends on surface details of code (e.g. does it adhere to linting standards/are the lines < 80 characters/imports ordered?) and how much time and effort a team spends on deeper architectural issues (loose coupling, de-duplication, un-reinventing the wheel, transforming imperative to declarative code, etc.)

Ensorceled 10y ago
Until the first time you spend a couple of hours looking for a weird bug in Python because somebody broke this rule...
- crdoconnor 10y ago
  Which rule? The rule that imports must be ordered according to the whims of the team lead? Or the rule that lines must be under 80 characters? There are a few linting rules which it pays major dividends to pay attention to (e.g. variables that are initialized but never used), but most of them are superficial.
  - Ensorceled 10y ago
    You commented on the import ordering so clearly that's what I'm commenting on. Not following the PEP standard for python import ordering can cause issues like circular imports or monkey patching not working properly.

jbrooksuk 10y ago

As a PHP "shop" this is why we use StyleCI [1]. Our code isn't that critical, the quality and maintainability are. We don't waste time on "code reviews" that only check the style of code, that can be automated away. Yes, I make StyleCI, but we use it in the place I work too (unrelated). [1] https://styleci.io https://styleci.io

dlandis 10y ago

Ugh, comes off as condescending, immature, and insufferable. I do think strict code review and high standards are the way to go, but when implementing them it becomes all the more important to have appropriate and professional interpersonal communication and documented policies.

louden 10y ago

'We are rigorous on code reviews' would be a better approach. We don't have to be jerks to make sure that code is up to standards.

jkot 10y ago

For me declaration of "ruthlessness" is a red flag, similar to ninja buzzword. Strictness in one direction does not mean company is strict at all fronts. Code reviews can be used without automated tests or version control. In this case the review process seems like a "busy work". Checks bellow could be easily integrated into build script or CI. Such code should not even made it into code review. (I am not arguing about the rule itself, just how it should be enforced) > You’ll notice that I left the comment “Beep!” on the imports of every file you touched. What I meant was, “Your imports violate our standard convention—we order them by built-ins, then third party, and then project level,” but that was too much to type on every file.

gdulli 10y ago

It's probably a joke but it's backfiring because most comments are taking it seriously and thinking the place actually works like this.

lcfcjs 10y ago

The code review process at my last job completely ripped the company apart. It has to be handled with a ton of consideration. Especially, if you are trying to communicate in comments on a Pull Request (and the developer is remote). It needs to be a learning experience, rather than a bollocking of a lesser individual. There is a good way to suggest changes, and a bad way. Developers are so often terrible at communication.

kvcrawford 10y ago

I miss working in an environment like this. I was actually surprised to find so much negative feedback in the comments. I think a detail getting glossed over is that the tone needs to be calibrated to the relationships between team members. If team members joke around with each other, send silly gifs, get lunch or coffee together, and are generally on good terms with one another...then it's known that you don't take PR comments personally, they're just feedback. And the more critical the feedback, the better developer you become.

Dommorak 10y ago
I can tell you that this is the kind of environment I work in and the first thing I teach a new teammate is that the team is only concerned with consistent high quality code. I think it is a good lesson for most developers and software engineers to separate their personal feelings from their code. It can help you be less defensive of your approach and take in multiple viewpoints to find the best possible solution for the task at hand.

cafard 10y ago

"One thing to keep in mind is that our customer base is unique. Our customers work for many of the most successful companies in their respective industries. They are where they are because they are committed to excellence and have great attention to detail." An awful lot of companies can describe their customers in such terms. Microsoft can, Google can, Pepsi and Bic can.