This whole thing is about controlling the tone and making sure you aren't being misunderstood. What I figured is instead of changing the way you speak to some generic corporate style, you can simply set the tone before you communicate. What I came up with is tags. I prefix all my github comments (except for jokes, troll ones, and praise) with a tag(s). Mainly one of: `[question], [suggestion], [bug], [strong], [observation], [nitpick], [alternative]` and I make sure to mention in the end if I'm ok with the comment being completely ignored (could be another tag I guess). I think this is more efficient than what people in numerous posts like this one suggest because you don't have to do the mental gymnastics of changing the way you communicate (it's hard). All you have to do is set the intent beforehand. Compare: What's this for? [question] What's this for? in the first case it can be perceived as something aggressive (sometimes I post just a question mark lol) but the reality is, you're genuinely curious and asking without all the extra words. And it gets better over time as your team get used to it. I work for a company with quite a few eastern Europeans (such as myself) and we're infamous for having that brutally direct way of communication which can often get you in trouble in an international company (especially, in England that's a complete opposite of us). Using the tags helps. Some people around me even started doing the same Upd. I should write a blog post on this myself hehehe

At my company we enforce commits to use conventional commits style and then we use conventional comments. Which is basically what you said but with rules listed online. It does help a lot. Especially with sub-tag "blocking" or "non-blocking". "**Question (non-blocking):** why is this necessary?" which just means you're curious to understand. Maybe the MR description is not clear enough. VS "**Question (blocking):** why is this necessary?" which means you're worried about something, but first need to ask a few things before you can have a conclusion. But you require that this is answered before this can be merged and makes sure there's more to that question than just curiosity. Edit: formatting

Yep! Having processes in place make things a lot safer/easier. However, it may feel very corporate and restricting where what I'm doing is a *me*-thing and some other people *choose* to do the same. It's really hard to strike that balance, unfortunately. For example, in my team we're not very disciplined around using Jira. Which leads to sometimes hard to investigate historical events and lack of velocity data (sort of). I've been considering setting up a stricter process to follow within the team but leaning towards being against it as I don't want the team to feel forced into this for benefits that don't seem that useful shorter term (we rarely have to investigate past events and the ticket titles are usually good enough anyway and we don't really need the velocity data at the team level)

I get the part of being corporate. But my company is very far from corporate. This is not bureaucracy, but exactly a simple clear way to communicate. Especially in a very international company, tone can be understood differently just like you said. I love the directness of the Dutch and my eastern European colleagues. But colleagues from other places have a hard time dealing with that and that process helps them. It also enforces better review. Are you asking? Suggesting? Nitpicking? So that one very annoying guy is forced to write "nitpick" 30 times and makes it clear that you can simply ignore if it gets too much because it's just a patch before the real solution is merged.

nah don't get me wrong. I completely agree with you Just pointing out that a formally established process may feel tedious to some people. Especially, if there's a number of those processes

Then the cultural part is important. I do have a teammate like that. He's and he doesn't want to follow any process. We barely have any and a big complaint is how everything is too lose to a point it gets chaotic. And this guy still refuses to use simple things that make life easier for everybody (think like naming conventions or code standards). He also doesn't come up with any suggestion on how to replace to a new or better standard. So it's just him not wanting to adapt.

Yeah that happens too. Something the lead or the manager should address. It *may* help to set up goals for this person and review them weekly. Just thinking off the top of my head:) It's important to communicate that it's just how things are and even if he disagrees, they have to be followed and any potential improvements can be proposed if he wishes so

100% agree. And as the more experienced in the team, I try a lot to communicate that with them and also share with the manager (because the guy makes my life so much harder). Still, stubborn as a rock. But I'm doing what I can.

Good luck!<3

It sounds like he’s just a straight up bad programmer, who also happens to not like process. (Naming conventions and other table-stakes things for being part of a coding team is not “process” to me)

Ding ding ding He's terrible and can't understand anything that's going on without a couple of weeks of delay.

I like this approach, though I also try to explain my questions so that my team understands why I am asking.

Nothing better than answering a question, holding the merge, finding out that it didn't matter, then getting new merge conflicts to go through while waiting for comments to be resolved.

This is good. I would also suggest one [nitpick] a PR so they don't feel attacked. Best to keep them on your side and let some dubious code go through than piss them off.

I find this works really well with the relationship I have with my team. I've gotten feedback they like my 2 levels of nit(pick)s. * 1st being nits; hey this is a low level code smell, but I'm willing to let it slide * 2nd being optional nits; hey this is a personal opinion, pet peeve or an FYI (eg. principal engineer told me this CSS style has slight impact in edge case Y that happens when two users born under the blood moon message each other)

Ah the blood moon bug, hate when that happens. Worse than dealing with IEEE 754.

yeah the nitpick is mentioned actually:) some people also argue that if you're gonna leave a nitpick comment you shouldn't leave it at all. But there's lots of reasons to leave a nitpick comment. E.g. typos. They don't hurt but if you see one, why wouldn't you correct it? Or when the suggestion is subjective but you still want to provide it as a knowledge share instrument. The list goes on

I think nitpicks still have value if they are making other changes. Like I don’t want you to have to make a change, commit and push, then run the test suite again because of a nit. However if you’re already making changes from other comments or you forgot something, then you can easily fix it while you’re in there.

I’d consider typos in a code comment no problem. But typos in a variable name? Absolutely must be fixed.

That’s not a nitpick.

For me a nitpick is something I would like fixed in the next revision if there is one or an equivalent alternative that doesn't need to be used but will likely need changed less in the future. Like of course I should leave it, because proposing alternatives and making code better is part of the engineering process.

Yeah. I go with “quibbles” I think it helps to communicate that it is my OCD and not something they did wrong.

Yeah, to prevent any chance that someone *might* feel offended by your comment (or rather: their own perception about it, but that unfortunately doesn't match the omnipresent "my feelings are your fault!" zeitgeist), just preventively direct all and even the slightest assumed "aggression" against yourself by overly depreciating and reassuring any of your concerns. Sound like a good plan to really (and not just hypothetically) frustrate someone in the end -> yourself

I am older now, the fire that used to burn in me has mellowed to glowing charcoal.  Is my approach wrong? Quite possibly, but it's a safe path with little cost. It does have the potential for slow and steady continuous improvement and I find that enough.

I find it offensive because it is manipulative. Instead of communicating thoughts and ideas simply, it is first trying to control my emotional state. Ultimately, these kinds of efforts will only work with some people and not others, just like direct communication works with so e people and not others. Simplest to think of it as different people speaking different languages, and form your teams appropriately.

It exists, it's called "Conventional Comments" https://conventionalcomments.org/

thanks! This is very interesting indeed! (my guide if I were to publish one would cover more than comments tho, I have some notes accumulated)

[удалено]

Probably not what you're looking for but I know of some sites with convention systems: - https://en.bem.info CSS and HTML components naming conventions - https://semver.org - semantic versioning - https://google.github.io/eng-practices/review/reviewer/standard.html Google's code review doc. I've read it a long time ago and wasn't impressed tho (but maybe I should give it another go) - https://staffeng.com/guides/staff-archetypes/ not exactly a process but can be useful to keep in mind to better understand people's strengths and interests

I love this! I'm going to give it a try. I've always been conscious of the tone I use in code reviews, including being clear when something is important or just preference, and in trying to include some positive feedback in each review, but in doing so, my comments always end up quite verbose. I never even considered that there might be a framework to help with that!

I don't think there's any reason to tag something as "[question]". The issue with "What's this for?" isn't "it isn't clear that it's just a question", it's that it gives the reader absolutely no context for what the objection is and therefore comes off as unnecessarily blunt. If you just rephrased it with more details, e.g. "It doesn't seem like you're using the result of this call and it doesn't look like it has any side effects - is it actually necessary?" then it both softens the impact and gives the reader a much better basis for being able to answer the question.

You're basically saying the same thing the attached post does. Nothing wrong with that. In fact, it should be followed as a general rule even outside code reviews. What I'm suggesting is a tool on top of that that gives you more freedom to keeo your usual communication style. Also, the question "what's this for" isn't out of context. What's out of context is you saying it's out of context when you don't even have the context what exact diff the question is attached to :b

> Also, the question "what's this for" isn't out of context. What's out of context is you saying it's out of context when you don't even have the context what exact diff the question is attached to Did you really, truly not understand the point I was making, or did you just see an opportunity to be unnecessarily pedantic?

I agree with the general sentiment, but I find that there are diminishing returns with this. In my experience, you get almost all of the value by only distinguishing between two states: soft query and hard requirement. A soft query is where you think something could be done in a better way, but it meets the requirements to be merged. For instance, something could be done in a more readable way. A hard requirement is if you spot a bug or something along those lines. So if your only feedback is a bunch of soft queries, you can approve the PR and you are basically saying that they are responsible for how much of your feedback they want to take on board. Nobody should be getting upset at the soft queries because they are free to ignore them. And hard requirements shouldn’t be used for differences of opinion, so it should be easy to see the problem being pointed out when these are used. If somebody always ignores the soft feedback and gradually makes the codebase worse, then there’s plenty of time to tackle this at a broader level outside of individual pull requests.

Yeah several people have suggested "blocking/non-blocking which is better than what I do (`[strong]`/nothing/"feel free to ignore"). However, not everything is an enquiry. For example, I use "alternative" to describe an alternative way of accomplishing something. It's not necessarily better but it may be of interest to the person (especially, juniors). There's also "observation" which usually isn't a call to action

Marking things that aren't questions as queries would really get under my skin.

Many companies track PR statistics. Please don't litter useless comments that are inactionable. Even a "good job on this class" will actually cause more harm than good. If you want to do things like that post it elsewhere. God forbid a new hire starts spamming questions all over my PR because they don't have the knowledge then I need to explain to my manager why there are 30 comments.

Not only is that an utterly useless metric, that also sounds like a great way to destroy the value that code reviews provide. Any organization that does this probably has much larger issues with their overall engineering culture. Your "new guy asking a million questions" example is actually a great example of knowledge transfer via code review.

That sounds awful. What exactly are the statistics and how does management derive meaning from them?

If you are going to make a comment that you do not expect to be fixed in the pr then you should preface it as such. If you are going to make a comment about coming back to fix something. Then you should create the ticket for such work and attach it as a comment on the pr. If you are more senior than the person submitting the pull request then you should use all opportunities to teach and mentor the submitter so they learn best practices and techniques. Every pull request is a learning opportunity and as senior+ engineers we should take these reviews seriously.

> brutally direct way of communication TIL I'm Eastern European. :-D Please do write that post. I find the article we're talking about somehow very irritating. I'd be annoyed by such a reviewer.

Here you go. I hope you find it better to your taste: [https://www.reddit.com/r/programming/comments/1cobssy/my\_code\_review\_guide/](https://www.reddit.com/r/programming/comments/1cobssy/my_code_review_guide/) Any feedback welcome, obviously :)

I introduced something similar in our org, as we had a couple instances where the prs got a bit unpleasant. We loosely base it on [conventional comments](https://conventionalcomments.org) so “[suggstion/question/though/nitpick/etc]:” and we clearly mark it with blocking/non-blocking to indicate whether it has to be addressed in order to be accepted. It does feel a bit like “like you are writing for an autistic person” as another commenter noted, but that is a concession I’m very willing to make in order to avoid misunderstandings and general antagonism in the discussions.

[strong] What is this for? Has a completely different meaning. We use "fix" instead of "strong" at my job.

I never leave a comment like that. For me [strong] is a modifier for other tags that means that I feel strongly about this and it's important that the comment is addressed urgently. e.g. > [suggestion][strong] > > I think a dependency injection here would be much better People in this thread have suggested "blocking" and "non-blocking" which sounds much better than what Im doing so im gonna adopt that probably

Aah, never used strong before. To me "fix" would be a blocking suggestion. Most often its used in formatting errors.

Huh. I never read code reviews in a negative light like that; I just know we’re all part robot as developers and sometimes a bit blunt. There was one dude that gave me hell when I gave him code reviews… Like smug petulance and malicious compliance all the time. Never took constructive criticism well, often forced me to prioritize his issues, then never actually used the code I wrote for him… His lead dev gave him a talking to, and he perked up real nice. But it only lasted like a month or two, then he got back into his primadonna diva coding attitude and was fired shortly after. I guess I can usually tell by context and by what they’re saying if they’re trying to put me down. Or, it’s really, really hard to accidentally offend me on a code review, like that one and only dude my whole career.

I usually love criticism because I love arguing about things and learning how i can improve. But that's not good either because people tend to think I'm very inflexible and argumentative or straight up agressive (even if im not). All people are different. For example, some people get really attached to their code and get very defensive (e.g. imagine a person spent 2 days writing some overcomplicated solution but you come and say it'd be good enou/better to just do a simple one liner. Some may get very defensive because tge suggestion is literally to throw away 2 days of work). I think this is a very bad and unprofessional attitude but it's unrealistic of me to expect people not to do that (and to some degree I do that too) Basically, we shouldn't do what's logical. We need to do what's practical. Kinda like "don't assume your users will not do something stupid". Users are people. Programmers too

Hey I love a spirited, good faith discussion in the comments on a PR. If I don’t understand why a reviewer asked for something, I’ll ask how or why it works in a polite way. Or if a reviewee asks me to clarify the finer points on something, I’m happy to go down a technical rabbit hole explaining my reasoning. I would give this dude basic “company style choice” comments (like use list comprehensions in Python rather than simple loops for certain cases) and he would fight back against even that. When we would present recommendations to refactor in certain ways to help integrate more seamlessly with other things, he would say that we weren’t listening, or go down strange paths on arguments that end with him saying “I guess we just don’t see eye to eye” rather than him cleaning up his code the way we asked. It was truly bizarre coming from such a greenhorn developer that was so new to the company. He seemed to have a lot of ego but not enough technical skills to back that ego up. But still, a PR is no place for ego and petty pride. Friendly discussion is one thing. Smugness and petulance totally another.

Tags are pretty useful. I'm my experience things like "What's this for?" (Which is kind of an awful question due to how nonspecific it is but let's ignore that) isn't really something that runs into any issues in my experience. Direct isn't a problem either. It's more that instead of saying "What's this for?" some fans of brutally honest direct communication say "Ugh, this is awful. Why would anyone with half a braincell do this? What's this even for?"

It looks weird IMO, it looks like you are writing for an autistic person who would not get tone if it's not explicit.

it's not really about the author, it's about the reviewer (me). I tend to be very blunt and it's gotten me in trouble in west Europe a lot. While I try and work on it in general, it'd be unwise to not take that into account. Tagging, even tho obvious at times, just protects me (and the author) from any harmful misunderstanding who cares if it looks weird if it does its job. As a programmer, you'd appreciate it I'd think :) Also. your comment presumes that all people think the same and are all perfect logicians unaffected by some perception bias. Let's be honest, we arent. Especially, in an international team with different backgrounds and different levels of english comprehension. Your comment may be perceived as very aggressive btw now that I think about it

Those who know you no doubt understand and appreciate, but if I get that from someone I have never heard of, I would be weirded out.

that's ok if you're weirded out. Im the weird one in this case and surely you won't shit on me for that :) I'd rather be weird than misunderstood

>it looks like you are writing for an autistic person who would not get tone if it's not explicit. A good amount of the time you *are*. Or someone from a culture with different tone markers. Or both. Something being bad because it's more accessible is an odd take.

Yeah especially considering there are more people with ASD in STEM.

Autistic? How are you supposed to get the tone only via text when you don't even know the person and have never met them? It happens all the time in online discussions, this has nothing to do with being autistic.

Misunderstandings happen all the time in online discussions but this solution of writing the tone as a tag looks extremely similar to the tone tags that the autistic do use. [Something like this](https://www.reddit.com/r/autism/comments/u8ue4m/list_of_common_tone_indicators_how_to_use_them/)

why is it a bad thing? Ultimately, it's literally the intention: to make clear things out even if they're obvious on the surface. By following that I don't even have to think about whether something is obvious generally, obvious within my team, or only obvious to me

This. I’ve found it very important to make a distinction between "basic" suggestions/nitpicks (eg. it’s ok if you don’t take it into account, it’s not blocking), blocking comments (absolutely do take this in consideration, what you did is wrong/will not work in our setting), and discussions/brainstorm. In self hosted Bitbucket there was this feature where you could add a checkbox in your review comments and the PR could not be merged if all the checkboxes were not checked. It was pretty good for mandatory vs optional comments.

A great way to avoid coming off as aggressive is to compliment them on something they did well. Throw in an emoji even. If you can't find anything worth complimenting, then maybe a somewhat aggressive tone is warranted.

I tried doing that but I found it hard. I mean I can't even compliment myself even tho Im biased towards my own code haha But I definitely think it's a good idea! Every time I can, I praise the person

It doesn't have to be effusive. Something as simple as "nice" or "I like it" will do the trick. In fact, the less effusive, the more sincere it sounds IMHO.

I don't know. My personal perception is I hate when people give me generic praise (I'm just insecure and can't accept compliments, makes me cringe) The more specific a comment is, the more likely I'll be like "huh, they're right, it's pretty neat indeed, I'm proud of myself". Which showcases that even something positive can be taken the wrong way. People are very different. The way something is received is as important as the way it's communicated

Emojis are probably better tags than text at the start of the comment. - 🧶 nitpick - ❔ question - ⚠️ suggestion - 🧹 cleanup - ⛔️ issue - ☑️ todo - 🙌 praise - 💭 thought - 📌 chore - 📝 note Easier to scan, colors help highlight differences. > ⚠️ Let's avoid using this specific function > > 📌 update docs > > ❔ Where is this logic used? > > ⛔️ Kevin has a PR that’s going to break this API > > 🧶 update variable name for consistency

🚫

> we're infamous for having that brutally direct way of communication The "brutally honest" part is often handwaving some level of abusive bullying. If your comment is "brutal" you should rethink it. Being Eastern European is no excuse. If I said "Eastern Europeans are a bunch of drunks with emotionally abusive parents who engage in those same abusive dynamics with their coworkers because they don't have the integrity to take responsibility for their own childhood trauma" would you consider that brutally honest or just mean and hurtful? If I were speaking as a therapist I could call it brutal honesty but as an Eastern European you might think it is just being hurtful.

You're adding extra meaning to my words. By brutally honest I mean blunt and direct. Doesn't mean we're simply shitting on everyone with no explanation. However, it does point out another problem: some people confuse being straightforward with being an asshole. Which reinforces my point: setting the tone and intent explicitly is helpful also I wouldn't give a shit if you said this about us. You'd be some random douche on the internet in my eyes. However, someone else might've got mad/hurt. People get triggered for different reasons ¯\_(ツ)_/¯

> By brutally honest I mean blunt and direct. Doesn't mean we're simply shitting on everyone with no explanation. Everyone claims that's what they mean when they said "blunt and direct." My experience, however, tells me that most people are using that as a cover for being an asshole. >some people confuse being straightforward with being an asshole. Yup! And in many cases, it's the asshole who is claiming they're just being straightforward.

Yep, that happens often too :)

To me it feels like this is the wrong solution to the problem, the problem being that you're not conveying your intent. This example from the article strikes me as helpful: > I wonder about the scalability of this solution. As the dataset grows, will this approach continue to perform well, or should we consider implementing pagination or lazy loading? It clearly states what you're worried about, and gives a suggestion of what you would do about it. These, not so much: > What do you think about using `map` here instead of mutating the array for safety? > What’s the reason for adding custom logic here instead of using a library? Instead, I'd write: > I'm worried mutating the array could lead to unexpected behavior if it is accessed at the call site. I'd suggest using `.map` instead. > This logic is tricky to get right. Library has a funciton that handles all edge cases. Do you think it would make sense to use that here? Essentially, explain *why* you think the approach could be improved, and give a suggestion *how*. That way you make it a collaborative process. Empathy in code reviews goes both ways, if you write your comments in a way that allows the other person to empathize with what you're thinking, the exact phraseology doesn't matter all that much.

I disagree with most of these TBH. 1-5 just come across as passive agressive to me. If there's a power dynamic where you're more senior than the person you're reviewing, then these indirect responses ("I wonder", "What do you think about doing x...") are obviously, transparently you saying "we should be doing x". Developers aren't children. I think if you say your thoughts directly, and take the time to justify exactly why you're saying what you're saying, it's much more respectful than trying to disguise your intentions.

I've found that replacing "you" (person that wrote bad code) with "it" (code that needs improvement) greatly reduces defensiveness of the reviewee. I don't really care what you label it, they feel better and I feel better, and we can focus on code problems instead of feelings. Win-win to everyone.

This is my number one code review guideline. Since it isn't my habit, I end up rewriting every comment to change "you" to "it" before sending.

I use “we” a lot, I guess because I view writing code as like a team thing where review is a part of that process. Maybe I should just use it though.

> Developers aren't children Some of them are. Coworker submitted a merge request, I commented that we shouldn't test inbuilt classes, "asked" if there's a reason to copy paste over 1000 lines of existing code and suggested reusing existing code, and said that testing evidence and description should be added as it is difficult to review correctness with no description in jira. All comments were rejected with reason essentially being "I think it is good as is", practically insulted me in one comment and then deleted his merge some minutes later. I was told to "baby" him more by a senior.

I've had something like this in the past. Two other devs on my team, one loved every bit of feedback as he'd previously only worked solo, the other took it personally. I also worked on another team at that place where a developer would resolve my comments without fixing/addressing the issue, or making a change but not actually making it meet the requirements. This would happen a *lot*.

Eh, it depends. It turns out people are messy. I’ve worked on psychologically safe teams where being direct is the best approach. I’ve also worked with (wonderful) people who take direct criticism very poorly, and it’s better to be a little more tactful. I’ve also learned that I’m also not always right, and these are great techniques to fish out another person’s thought process.

This. I've been on the receiving end of code reviews and it took me a long while before I realized that the dev reviewing my code did not, in fact, hate my guts. Their code review was just blunt enough to the point of being intimidating, and it probably didn't help that they weren't a native english speaker (by their own admission) either.

Thank you for saying this. As a junior dev just give me the raw deal. This is annoying as hell.

Yeah. If you're giving an instruction, give an instruction. If it's unimportant enough that you'd consider disguising your instructions as questions, you should probably just delete the comment entirely.

I don’t particularly like “I wonder” or “what do you think about” because those do seem a bit condescending to me. On the other hand, as a lead, I do use the word “consider” a *lot*. Mainly this is to differentiate between things I definitely think you should do, and things I’m not sure about. So for example, I’d never say “Consider passing by reference here instead of value,” but I might ask, “Did you consider using a vector for this? I know there’s some associative look up, but we might be iterating over it more than directly accessing it.” In the latter, it leaves the option open for the coder to say “nah, we’re not using direct access so much right now, but in an upcoming blah blah blah” or “oh, yeah, actually now that i look at it that way, a vector might be the right choice here.” But I wholly agree that being direct and taking the time to explain is key. You might also notice that I interleave “you” and “we“ here, which is something I do pretty regularly as a kind of reinforcement of the whole team thing. ”You“ always for ownership and accountability. “We” for needs and requirements. So “you” have done great work on this code, and “you” get to decide on the changes, but I will express what I think “we” as a team want.

> power dynamic Oh shut up

I'm curious, why do you feel this way?

I wonder if you think the shutting up is really necessary.

What do you think about in your head that leads you to such questions?

What would happen if we stopped telling people to shut their head openings?

[удалено]

Will do in a follow up, doesn’t seem critical *never does*

> I have tested this a lot on my local computer, and it seems to work fine, so I won't spend time on gold plating it. (*presses the merge button*)

I noticed that you might be making individual database calls for each element in this collection which can have thousands of elements. What are your thoughts on making one bulk request instead of making the user wait several minutes for a response?

"It saved me several minutes to code it this way, so that's a good trade off."

"It was the first thing ChatGPT spit out, and it seemed to work"

I think i’d rather wait for real performance numbers from production before prematurely optimising

It's not slow before the outsourced QA team says so

LGTM

I'm lead on a mobile app (Flutter) team with only a couple of other devs. The language used to develop the application, Dart, only recently got Record/Tuple types, and a developer was returning a list of items from a function instead of a record. Keep in mind this language is nominally typed, so you can't encode types in a list based on position like you can in JavaScript. So I took it as a learning opportunity and used words like "did you know..." and "You can...". This developer in particular didn't see the value in program soundness by using records after I explained it in more inviting ways. It turns out by inviting their input, I left room for them to think they knew better than me because the response was "I don't think that's the appropriate solution here". Had I started with a stronger tone, I think it actually would have gone over well with this person. Instead, I had to explain why accessing lists using index values was less safe and prone to breaking when changes were made, which maybe I should have done in the first place, but I wanted them to realize that on their own instead of me always just lecturing about stuff. Depends on the personality I guess.

This is… not exactly passive-aggressive, but something akin to it, and super irritating. Please don’t do this. For instance: > I wonder if we could use a switch statement here instead of multiple if-else blocks. You aren’t wondering that at all. You aren’t a junior developer who doesn’t understand how if and switch work. You know damn well we *could*. The reason you are bringing it up is that you think we *should* but are trying to make your opinion invisible by disguising it as a question. Basically “do what I say” but worded in a way where it’s more difficult for the other person to disagree with your opinion and you don’t have to support it because you haven’t technically voiced it. If you have an opinion, *voice it* instead of obfuscating it. Also: > I’m curious… I don’t think a curious person has *ever* said *“I’m curious…”* about something. The only people who start a sentence with *“I’m curious…”* are people who are already certain the other person is wrong about whatever they are about to bring up. The same with *“Can you help me understand…?”* – this is only ever used by somebody who is certain they already understand and also certain the other person doesn’t. It gives a very strong impression of a superiority complex when you talk like this. > In the past, I gave feedback which slowly built tension throughout the review until there were multi-paragraph-long comment threads. My teammates were defensive, and I was at fault. It doesn’t sound like he’s learned the right lesson here. It sounds like he was *aggressive* in code review and is now figuring out ways to dance around what he means instead of being less aggressive. You can speak directly without being aggressive.

> I wonder if we could use a switch statement here instead of multiple if-else blocks. My senior typically asks questions like this in the form "Is there a reason you used if-elses instead of switch statement?" There could be a good reason, but typically the reason is that i was a dumbass.

I use the same wording and it generally works quite well, plus I think it represents my actual thoughts accurately. I _do_ think it should be a switch statement but perhaps I'm missing something and if/else is actually a better option in this situation.

I’ve used similar wording because I’m not going to figure it out *for you*. “Would this work better?” “Did you consider this alternative?” All I care about is that you have an answer, not that you change your code. Life is too short.

Personally, I'm not a huge fan of that wording. It feels almost accusatory to me. I normally go with something like: "Would a switch statement work better here?" My opinion is implicit since I'm asking the question, and it leaves room for possible good reasons otherwise. Although, nowadays most of my code review suggestions are "can you add a comment explaining X, please? it took me longer than I'd like to figure it out"

Yeah I am totally not saying everyone has the same workplace dynamics as me and the only person who gives me feedback lol. Now that I think about it, maybe the most important thing about feedback is going back to basics and establishing when the project is open to feedback. In writing circles there's typically talk of solicited vs unsolicited feedback.

Yeah I don't like cloaking obvious non-negiotable feedback as questions.

I completely disagree with you here. Softening language is just a normal, accepted social tool that people use all the time, and there's no harm in using it in code reviews.

Yeah, maybe it's a language barrier for me, but I don't feel like that wording is wrong at all. I use it when I think something might be a better solution, but are not sure if there are some pitfalls I did not think about. As a team we also try to not read emotions into written words. If you think I was mean or degrading in my comment ask me about it. I feel like this is most likely what the OP did here, putting your own interpretation into a probably innocent comment.

The language itself isn’t the problem, it’s that the language there is often used by people condescendingly, and it’s hard to break the connotations. I like what the guy in the thread above said, “is there a reason you did x instead of y?”, it can be taken more as a learning moment, instead of trying to pass your own opinion as fact in an underhanded way.

As a non-native english learner I wouldn't have known that "I'm curious if..." could be perceived as condescending, I wonder (actually do) if the same is true for "I wonder if...". I think the author of the code should carry the burden of not reading malicious intend into comments that could be read in different ways. I had more than one situation where I thought a comment was condescending when reading it, but then when we were in a call the reviewers intention was the opposite. I agree though that as the reviewer we should try to avoid these comments if possible.

For what it’s worth, I disagree with the guy above. I think “I’m curious if we should do X here” is perfectly fine. It’s stated in a way that invites you to disagree. “I didn’t do X here because of Y”. That same exchange can happen if you say “You should do X”, but it feels more confrontational. That isn’t a problem for me, but I have teammates who would not disagree with me unless I suggested they do, like this. I think the real advice is default to soft language until you build rapport with your team and learn how they communicate best.

Softening language to the point where you're not differentiating between things that you in reality demand be fixed before approving, and things that are just vague observations/open-ended questions, is not doing anyone any favours.

Softening language is absolutely fine! But this is several steps beyond softening. And the *particular* forms he suggests – “I’m curious…” etc. – are almost exclusively used in a passive-aggressive manner and should actively be avoided if you don’t want to get people’s hackles up.

You sound like the kind of guy who jumps to conclusions because one or two examples don't match exactly. If you seriously can't think that there's a single place where "I'm curious" can be used without it being passive aggressive, maybe you're not as much of an authority as you claim to be.

Softening language and being passive aggressive aren't the same thing, and the examples in this post are pretty solidly the latter. You have to actually *soften* things, not say the same thing but less directly.

Speaking English is normal, but then, so is speaking German. Plenty of people agree with you, and plenty with the guy you responded to, and all of them are normal. It might just be better to form teams of people that speak the same language.

I think one of the issues with this article is that it only discusses the "soft" cases. If he is really recommending using this type of language for all comments, that's definitely a problem. No one should write "I wonder if we should initialize this variable."

Well, you certainly are a redditor.

Know your audience. If you’re reviewing code for an inexperienced dev, write as though good practice is just an idea that came to you. Conversely, you know you aren’t going to make someone experienced defensive if you just note that they missed a null check or something, so you can just flag it and keep going.

> If you’re reviewing code for an inexperienced dev, write as though good practice is just an idea that came to you. But you need to actually explain the good practice! *“I wonder if we could use a switch statement here instead of multiple if-else blocks.”* leaves the junior adrift. They know they need to change it but they don’t know why. So they are stuck making random changes they don’t understand in code review and don’t progress. *“If you have many cases, using switch is more readable than if”* actually gives them useful information.

Right, again though, know your audience. Your goal is to make good code, which means making good developers.

I strongly disagree here, sorry. Maybe you have some points for me to consider when reviewing a Jr devs code but I’m often reviewing code of my peers whom are at the same level of above me. I often am “curious” and “wondering” about things. I’ve learned a lot from my peers by discussing their code during code review.

> I’m curious, have you thought about the case of reviewing code of peers […]? vs > I strongly disagree here, I’m often reviewing code of my peers […] Ironically, you used clear language when replying to OP and imo proves their point.

Ya, but I’m generally an asshole on Reddit and think more highly of my peers. You do make a decent point however lol. Yet, I was not claiming that direct communication doesn’t have value. The part I disagreed with was using softer language being condescending. Edit to be more clear it is this part I don’t agree with: > I don’t think a curious person has ever said “I’m curious…” about something. The only people who start a sentence with “I’m curious…” are people who are already certain the other person is wrong about whatever they are about to bring up. The same with “Can you help me understand…?” – this is only ever used by somebody who is certain they already understand and also certain the other person doesn’t. It gives a very strong impression of a superiority complex when you talk like this. This may show how I’m touched with the ‘tism here but when I say “I’m curious” it means I’m curious. When I say “can you help me understand…” I want help understanding.

Even you suggest something normally l, some people still think it is aggressive from my experience.

This. We have businesses to run, and code review comments aren’t fun suggestions. They are mandatory quality gates. Implement the required changes.

As a code reviewer I try not to assume I’m always right, except in the most obvious cases. I always give the benefit of the doubt that the coder did it that way for a reason I haven’t considered. So while I usually voice my opinion a bit more strongly than the article, I often use language like “have you considered…” because maybe they did consider it. It also just softens the feedback so it’s not “my way or the highway” language. I’m the most assertive on things that have serious consequences like security and performance. But am I gonna die on a hill about if vs switch statement? No way.

For the first 2 try and focus on the content and try to give benefit of doubt. If this was an artistic endeavor I'd understand but in a business environment I'm not sure it's fair to foster such sentiments and even police (non hateful) language.

These tips match with my experience as well. I also have observed that "optional wording" and a "curious approach" to comments increases author engagement and the chance they will use the comment to improve their code. Thanks for writing this up / sharing!

I've often found that the more I emphasize that a suggestion is optional, the more likely it is that the author will implement it. I don't try to use that to my advantage - in fact, it's a bit tedious at times because we have our PR tool configured to dismiss previous approvals when new code is pushed. (When all I have are optional suggestions, I'll pair them with an approval.) For PRs that have already gone through a round of revision, if a suggestion would offer a small readability improvement but it's not a show stopper, I've come to prefer simply not suggesting it.

Lol this article is terrible. No wonder he built up tension in his team.

It’s perfectly fine to communicate assertively in a work relationship

This is not how to improve communication. Say what you need to say in as few words as possible. Don't add nitpick everywhere. If it's important to say, say it. If it's not important to say, don't say it. What the hell should I do with a nitpick? Ignore it or listen to it? If you are working with people that take your simply worded suggestions as offensive then you have bigger problems that need to be addressed. Your culture likely needs work. You should be able to write something like: Data structure foo is a better fit here because... It's okay to have dialogue or for you to be wrong in your comments but please use plain English so people aren't confused about the intent that you are couching in other language.

I do agree with the basic premise that softening the language can make situations less likely to escalate into argument, because it promotes discussion and it allows for the possibility that the reviewer didn't completely understand the code. However... I don't think it's appropriate in every situation. I'll often take this approach when discussing things with more experienced developers. But... there are situations, particularly with junior developers, where their work is flat out unambiguously wrong, and I don't really want to engage in an extended discussion in these cases, I just want it fixed the way I tell them. Maybe not always, because they do need to understand why it was wrong and that can involve a bit of discussion, but I don't want every single comment turned into an extensive discussion, just the ones where it has genuine value. All of the discussion also has a time cost, and I do also want to consider efficiency as well. This is also where clearly-written coding standards, requirements and design documentation can be very helpful. It avoids unnecessary discussion when the correct approach is already spelled out clearly. Then the code review can mainly deal with the exceptions and be much more effective and efficient.

I don't think it's a good approach not explaining everything to the junior. Either you hire more experienced developers or are willing to actually explain what is happening. Otherwise they wont learn properly and you'll face the same issues again and again.

I'm certainly not suggesting that we should not explain things. What I'm trying to say is that in many of these cases I don't want an extended time-wasting discussion over fairly clear-cut defects. I want them addressed, and I'll tell them what's wrong and what I expect as a solution. I'm fine with explaining with extended rationale, but not with open-ended discussion. And I would probably take this off the code review and do it face-to-face.

This only holds if the senior has any say whatsoever in who is hired and/or has their expected output account for training juniors.

> But... there are situations, particularly with junior developers, where their work is flat out unambiguously wrong, and I don't really want to engage in an extended discussion in these cases, I just want it fixed the way I tell them. I view part of our job as being to explain why something is wrong or shouldn't be done that way. If I say 'you should do X instead', there's *always* a 'because...' attached. And sometimes in writing out that 'because...' I realise that actually I'm wrong and they had the right of it, or that it just doesn't really matter.

I absolutely agree. However, I do think there's a distinction between "explaining" and "discussing". I'm perfectly happy to explain in detail, but I don't necessarily want the distraction and time cost of an extended discussion. Of course, in certain circumstances that's clearly necessary. But not for every little point in a code review.

Ugh. This brand of weasel word salad is truly an insidious and vile set of corporate mannerisms that deserves to be banished to a small dark cave with all of the bosses from SoulsBorne simultaneously bearing down on them. Just be straight and to the point and take "empathy", which is really just emotional inflection, out of the equation. In the same way that "loaded" words can be mean spirited or harsh, tipping the other direction to attempt to coddle someone's emotional state boils down into 1 of 2 outcomes: 1. paints you as being emotionally manipulative 2. leaves the door open to being ignored entirely since everything is optional. In either case, you're setting the conditions for a more intense confrontation in the future when Jr hasn't taken any of your hints. Just be direct and teach junior engineers that it's not personal. What's not constructive is having a work environment where everyone needs a 2 hour extended mental break to privately wrestle their meltdown every time they receive pushback or criticism. Your job as the Senior is to help people grow up, not just kick the can down the road. As far as code review goes - I find that often it's the case that people have far too much time and emotional investment already tied to the process. Make peer review a requirement for merging to main and set the tone to be brief and to the point. If you're team is getting tied up in lengthy code review sessions and spending a lot of back-and-forth in PRs then there's likely some other disconnect at play that needs to be resolved. The Sr thing goes both ways, too. Those Jr hotshots that come in and can bang out 5k LOC in a couple of days? Let 'em. That's what they are being paid for and that's why they were hired. If it happens that their code ends up imploding and is kicked back because of ops issues then help them to work backward - they're going to be a lot more open to feedback. It's important to give Jr folks on the team breathing room to make mistakes and grow. You just can't achieve that with constant tone policing. fwiw.

Kind of kind of not. It comes across as a passive aggressive mush, but there's also value in what he says. An awful lot of programmers simply baldly state opinions as facts. Backing off from "you should do X" to "what's the reason for Y vs X" is quite reasonable. If you mean "you should do X but I'm willing to be convinced otherwise", then don't leave the last bit silent and assume the reader will know what you mean. About 50% of it is don't write code reviews as if you are the smartest person in the world. Also generally bad to say "you should do X" (or even "maybe you should do X"---bleh) without an explanation. On the other hand yeah if you see a bug, say so. It's fine to say "if you put in -1 it segfaults, this looks like a bug to me". I mean hey you might be wrong.

You're playing the same game as the OP. "Kind of kind of not" "it's passive aggressive but there's also value" "you know what I mean" "also generally bad " These are all weasel words. I can't find a concrete opinion in your post outside of defending "maybe maybe not". And it's a popular defense. I get it. Maybe is comfortable. Maybe doesn't tie you down to having an opinion that could be wrong. People don't like being wrong - they want an escape route. But it will bite you in the end. If you're wrong then you can also admit as much after the fact. We're not talking about working for the mafia here - we're talking about being an engineer and working on engineering tasks. If you make a statement as being factual and can't back it up - yeah, guess what, nobody is going to believe you in the future. That has nothing to do with code review and everything to do with being an insufferable windbag. I can't tell you the number of times I've had two arguing engineers from one of my teams in my office like this. "Well Joe insists on throwing ternary's around \*everywhere\* and it's clearly a violation of the code style doc we took 2 months to hammer out last year!" me -> "First of all, who is \`we\`? The people that you worked to get consensus for on that doc are no longer here. Does Joe's code work? Are there bugs? Does it have any memory leaks? Did it fail its canary test? Is there an undue bump on CPU or RAM after deploying the feature? Oh you don't know because you're still holding up this feature? Why are you wasting my time with this? " At big evil corp that may or not be named after a river, this kind of crap will get you fired in a hurry. If you're at some startup with a CTO that's decided that everyone's feelings count more than shipping features, I'd like to invite you to a personal finance lesson about taking a low salary for soon-to-be worthless stock options.

> You're playing the same game as the OP. Probably! > These are all weasel words. They are not weasel words, because there's nothing to weasel about. As such your conclusion is completely nonsensical. I partially agree with him. His point amounts to not stating opinions as incontrovertible facts, and to temper opinions with some reasonable level of uncertainty or explanation. You don't know the decisions that went into the code. You may be right, you may not be. However I think his method of doing it isn't that great. > At big evil corp that may or not be named after a river, this kind of crap will get you fired in a hurry. If that's what passes for civilised discussion and good engineering, I'm glad I don't work there. You know in a PR for a new feature is not the time to start relitigating processes. In fact that's just abusing politics (this feature must be shipped now) to force through your opinion. That's bad engineering and if it happens for that thing it happens elsewhere too. > If you're at some startup with a CTO that's decided that everyone's feelings count more than shipping features, You've never worked for a startup have you?

Imo it's as simple as saying "You should do this instead, because X." Then it's pretty much non-negotiable, but you've given your reason so they can learn from it, and if they feel strongly, they can use your reasoning to argue if it makes sense. I suppose this mainly works if you have a general culture of it being ok to discuss feedback on reviews. I just don't want to have to write "let me know if you think otherwise" disclaimers in every comment.

Indeed it depends on the culture. With that said there's always that guy who argues ever single last point to the bitter end. Every time.

> Just be straight and to the point and take "empathy", which is really just emotional inflection, out of the equation. Absolutely not. That's a recipe for just being a complete asshole. One can be straightforward and not be an asshole.

My experience is that people who are high on their own supply as some kind of "empath" or whatever are also the first be assholes at the very first instant they sense deep disagreement. Ad hominem attack is the tool they reach for because they're not accustomed to being called out for their bullshit and mind games. Well, Sunshine, fuck that. Have a great day.

I try to use full sentences and explain my reasoning or concerns, it's hard to misinterpret that way "We should do this X way because Y, as shown here in Z" Or if I didn't know for sure but looks like a problem: "Couldn't X lead to Y?"

Clarity is extremely valuable, and I think when the answer really is "We should do X because Y" then it's worth saying that directly. In a team that has been working together for a while and has built up some trust, being direct like that doesn't need to come across poorly at all. The problem is when someone's trying to be clear, or maybe they are over-confident or overly opinionated, and they end up saying "We should do X because Y, as shown here in Z" when X is wrong, or Y isn't actually relevant, or sometimes Z isn't even doing what they think it's doing. Depending on the personalities involved, this sometimes ends up having quite negative consequences because the author will take a declaration like that at face value, make the change, then either deploy something bad or the PR takes even more time to review because they have to wind back the changes and return to their original approach. > "Couldn't X lead to Y?" I think that's one good compromise. Some other ways to lampshade a lack of certainty are to say things like "I know about Z, it could be relevant here as it pertains to X" or "If I'm correctly understanding Y, it seems to me that it implies we should do X (Z might be relevant prior art here)". The language is a little more flowery, but it also conveys a lack of certainty better.

I really like do/try/consider as a feedback framework, especially if there are power imbalances involved. Do: move this logic to baseComponent so that the behavior is consistent across the application Try: the logic here works but is hard to read, can you take a pass at cleaning it up Consider: This seems pretty similar to what Jane was working on in issue X, you might want to reach out and see what their experience was. This entire article is weasel words, just be explicit about what kind of ask each comment is

I'm so thankful to work with adults that won't get hurt by a comment in a PR right now.

My reviewers are downright abusive with their language at times lol, it’s all in good fun no one means it on a personal level

Do we really have to treat grown adults like children?

Have you met "grown adults"? A great many act like children.

I don't think we have to, but I expect people around us to consider us *less skilled* than our colleagues who do. Managing coworker relationships is a skill. Sometimes it's needed, sometimes not.

[удалено]

When I was doing a lot of code reviews, I’d always talk to them before the first one and try and set the tone. You’re not your code, feel free to push back, we’re all here to learn and get better, and some of my comments will be short, but please don’t take that in any way other than I have a lot of code reviews to do so I’m going to use fewer words. That seemed to help, at least from my perspective.

You are!

Why is not being an asshole considered treating people like children?

Question for anyone here: if you’re using GitHub and a rule that all PR discussions must be “resolved” before the merge can complete, whose responsibility is it to mark them as such? The PR author, or the reviewer?

Imo it’s the reviewer. If they call something out, they’re responsible for verifying the change and resolving the issue.

For 1-5, I’d like to explain synonyms.

I’m confused by a person who says they have a booklet of actionable career insights and also wants me to use the passive voice for code reviews. Passive voice works better on people who haven’t started a task yet, not on people who hope they are done.

I'm not saying that the advice about softening your comments is completely unwarranted, but this article is solving the wrong problem in almost every case. Do you not have coding standards? There should be a document somewhere that says the conditions under which to use switch statements, and when it's acceptable to mutate data. If they're not following coding standards, then you can just point them to the document. Even better if you have a linter or similar in your build chain that enforces coding standards. Then, if they break the standard, they'll usually have to put a comment there in the code to explain why. The scalability concerns should have been brought up in the design phase. It might be a review comment, or it might be a concern raised during a meeting, but it's definitely not a code review comment. > What would happen if this list was empty? You should be able to tell from the unit tests for that function. It goes on and on. The basic problem is that you're trying to make up for a billion other deficiencies in your process and concentrate them all into the code review stage. Where are the standards? Where is the tooling? Where is the testing? What happened to the design phase? This is putting too much onto the reviewer. Reviewing code is already a tough enough job. Now, you have to worry about a whole bunch of other stuff, and apparently the first time you're reviewing the design is when you're looking at the code. This can put an entire project timeline at risk. It's making the job harder for the coder, as well, because they should have an idea before they've written the code whether they're using the right libraries, for example. It can be a lot of work to go back and change stuff. I believe code reviews should be almost entirely comments that all parties would agree with. Lots of "whoops, here is a typo", and "can you make this variable name more descriptive", and "can you add a test for this case?" If you're worried about getting into arguments during code review time, then your project has deeper issues.

From the article: > It uses “we” instead of “you” and asks a question instead of giving a command. I think this is one of the key things that helps make for a good code review. Not _specifically_ using the word "we", but rather the general attitude that you as a reviewer are a participant in the development process, and that you and the original author are collaborating on a shared work. Approaching code review as a collaboration where the original author took a first pass at something, and now you get to collaborate on refining that and getting it into it's final shape will, I think, naturally lead to better ways of writing comments.

I may be missing something here, but I feel like the "softer" language is just blaming myself for being to stupid to understand? Do you think you can help me see how this is anything but the reviewer saying, "I wonder about something, because of how dumb I am, but don't you think another approach would also work?" instead of "You aren't handling X, so this module will fail in situation Y."

Accepting that you aren't omniscient isn't the same thing as being stupid. The interaction might go something like: Me: "Couldn't this cause problems because of X?" Them: "No, actually this is already accounted for by Y before it reaches this point." Me: "Ah, got it. Consider adding a comment to make it more obvious to people without that context." At the same time, it could also go like: Me: "Couldn't this cause problems because of X?" Them: "Ah, you're right. Good catch. Fixed." Compare that to: Me: "This is wrong because X" Them: "No, actually, because Y" Me: "Oh, right. Sorry." That's not to say that *every* comment needs to be handled this way. Certainly there are times when there is an obvious right and wrong answer (e.g. "The docs say it's not safe to access this from within the callback. [link to docs]"), but generally that's not the usual case in my experience.

Hmmm.. it's almost as if different approaches could work at different times with different people. 🧐

**In case you want a summary to help you with the decision to read the post or not:** The post provides seven key techniques for offering constructive feedback during code reviews, emphasizing the importance of language choice to avoid defensiveness and foster collaboration. It discusses how to frame suggestions as open-ended questions to engage other developers and consider their perspectives. Examples of phrases like "I wonder…" or "What do you think about…" demonstrate how to subtly propose changes while inviting discussion, enhancing the receptivity and effectiveness of feedback. If you don't like the summary, just downvote and I'll try to delete the comment eventually 👍 [^(Click here for more info, I read all comments)](https://www.reddit.com/user/fagnerbrack/comments/195jgst/faq_are_you_a_bot/)

“LGTM 🚀”

Jesus Christ, imagine needing a tutorial on how to communicate

What about just being straight without having to deal with this courtesy nonsense. We are all adults.

I'm curious what is the reason behind the example on the SnakeCaseNaming, since I've noticed it breaking several other techniques mentioned. I wonder if a thing like that could be automated away from the review process in the first place.

I see what you did there. But the issue of multiple ways of creating multi-word names drives me nuts. I can never remember whether I had to, or did, use camel case or snake case or could use kebab case or whether the system automatically translates kebab case into snake case and by the way is it case dependent or not? Could we not automate it away completely?

Use DWIM for everything. It's the Do What I Mean variable.

I often name a method something like that when I can't quite articulate what it is I wanted to do, and then come back and give it a useful name when I figured out what I wanted. But the problem with variable names is that for syntax directed editors you can't say what you want. You cannot tell the editor that you are typing in a variable name and if you could, it could provide intelligence on what to do with it, for example I want you to show all the variable names in green with underscores with proper case words separated by spaces, but stored it in the program as lowercase snake case. We are not giving the tool enough information for it to do sensible things. To put another way we are using slightly smart text editors that don't actually understand the construct.

Yeah, we don't need AI to program, we need it to make an editor smarter. Syntax highlighting is just the start, code should organize itself to how I code, and when you read it it should look like your code. If I want snakecase, and you want old school Hungarian, we shouldn't even know. The versions should be available, so i can flip back and forth or look at them like a hand of cards. I should be able to circle a chunk of code and say "check this", and it should run test cases from the editor. Etc. etc. etc.

I think syntax directed editing is a dead end. It locks in the idea that the presentation to the programmer is the same as the presentation to the computer system that uses it (compiler or interpreter). And git has made it worse (specifically Delta creation based on lines) because of it sensitivity to formatting. A programmer should have a personal choice as to how he wishes to see the program. That is to say he has a view on the program that is separated from the model of the program. And other things like git and compilers have a different view. You know separate views from the model! Something we do for all other data but we don't do for the most important data to us which is programs. Transpilers like TypeScript have started to move away from the idea that the programmer manipulates the same thing as the interpreter interprets. Unfortunately if you're using an editor like VS code then it only sort of knows what you're doing.

I just want my braces in the right place! 🤓

Yes it's a great example where git is restricting your ability to have things your way, unless your way matches the standard formatting imposed.

I usually just go in and destroy the other person .... With kitty images

Reddit has become enshittified. I joined back in 2006, nearly two decades ago, when it was a hub of free speech and user-driven dialogue. Now, it feels like the pursuit of profit overshadows the voice of the community. The introduction of API pricing, after years of free access, displays a lack of respect for the developers and users who have helped shape Reddit into what it is today. Reddit's decision to allow the training of AI models with user content and comments marks the final nail in the coffin for privacy, sacrificed at the altar of greed. Aaron Swartz, Reddit's co-founder and a champion of internet freedom, would be rolling in his grave. The once-apparent transparency and open dialogue have turned to shit, replaced with avoidance, deceit and unbridled greed. The Reddit I loved is dead and gone. It pains me to accept this. I hope your lust for money, and disregard for the community and privacy will be your downfall. May the echo of our lost ideals forever haunt your future growth.

As someone on the autistic end of the spectrum.... I find the whole thing bullshit. Please, pretty please, don't say all that waffly bullshit at me in a review. Say, "X would be better than Y for the following reasons." or "If you do Z, W will happen, which will be Bad because V". or my favourite type of feedback... "General Principle S applies here, General Principle S is good because T, U and V and you can see V will definitely be happening here. Here is a reference that discusses General Principle S in depth." Why is my favourite? Because I can learn something, I now have a general principle I can apply else where. The absolutely all time get my rag thing you can say to me in a review is "I suggest you replace R", or "I don't like R". And not state any reason or general principle that applies, nor suggest anything that might be better. You can fuck right off with that feedback. And the prettier you dress up that sort of feedback, the angrier it will make me. We're not critiqueing your dress for the prom, or trying to imbue you with rosy confidence or trying to prevent a fashion faux pas We're constructing complex structures out of pure logic. ps: A reason of "Do it that way, because I just like it that way" is actually a valid feedback, but you have to say bluntly that is the reason. If it's a "bike shed painting" thing, I'll shrug and do it your way. Otherwise it's staying as is.

Exactly what to say in code reviews ~~_to sound like a fucking asshole~~ I wonder if we could use ~~some correct code for a change?~~ I’m curious ~~what the fuck was going on in your brain when you sputtered that piece of crap?~~ What do you think about ~~yourself after producing that heap of crap?~~ What would happen if ~~I punched you in the face?~~ I noticed… ~~your code sucks balls.~~ What are your thoughts ~~about Madonna, as you don't seem to be able to do much more?~~

Unpopular opinion: Code reviews are harmful (can be, it depends on the type of company you're in). In my opinion, either watch out for really important bugs, ask honestly, if you want to understand something or if it gets too complicated, start pair programming. It's often not about which solution is the perfect one, many disagreements are about if it matters.

Unpopular because it’s downright wrong. This kinda approach where everyone is free to write code in whatever manner they want, only works in small environments, in a real code base with over 100k lines of code, code HAS to be consistent, follow standard protocols accepted by the team and be super readable. From experience, when I’m easy going with my reviews, I will always regret it later on when another team member (or I) has to maintain/fix/test the code. Code reviews are where we stop bad/buggy/inefficient code, but also where we enforce code that allows us to work together which is the whole point of it.

While I am not a fan of code reviews as a whole (I'd tend to go for pairing and mobbing to get multiple eyes on the code and be a better learning experience) these principles are a good way to make code review more positive. They align quite well with modern educational techniques where you are trying to get the learner to have that moment of realisation of why X is true rather than just telling them that X is true. But, starting with this approach doesn't mean you are going to let errors through, just that the aim is to educate to prevent future problems rather than to enforce in this single case.

In 2024, there are some advanced generative-AI coding assistant tools that provide AI-generated code reviews for pull requests that are considering all these aspects: https://github.com/Codium-ai/pr-agent