site banner

Wellness Wednesday for March 15, 2023

The Wednesday Wellness threads are meant to encourage users to ask for and provide advice and motivation to improve their lives. It isn't intended as a 'containment thread' and any content which could go here could instead be posted in its own thread. You could post:

  • Requests for advice and / or encouragement. On basically any topic and for any scale of problem.

  • Updates to let us know how you are doing. This provides valuable feedback on past advice / encouragement and will hopefully make people feel a little more motivated to follow through. If you want to be reminded to post your update, see the post titled 'update reminders', below.

  • Advice. This can be in response to a request for advice or just something that you think could be generally useful for many people here.

  • Encouragement. Probably best directed at specific users, but if you feel like just encouraging people in general I don't think anyone is going to object. I don't think I really need to say this, but just to be clear; encouragement should have a generally positive tone and not shame people (if people feel that shame might be an effective tool for motivating people, please discuss this so we can form a group consensus on how to use it rather than just trying it).

5
Jump in the discussion.

No email address required.

Where are my fellow software engineers at?

How do you all deal with not taking PR comments personally or not allowing yourself to get frustrated by them?

It seems like half the time I have to end up arguing over minor bullshit with colleagues that boils down to “I wouldn’t have done it that way” or “it’s not absolutely perfect” or “did you consider doing it this way?” (This way being probably the more “idiomatic” but often far less readable way).

Obviously I’m not going to get offended if a colleague catches an obvious mistake I made, or asks for more documentation or comments because something is unclear, but usually it’s what I described above.

Do I just need to develop thicker skin with this shit?

I've only sometimes run into this and it's usually with people who have some personality issue, so I'd wonder if there's some culture thing happening where you work. I am probably more nitpicky than most, and I know more and less nitpickers where I work, but nitpicks are usually brought up and dropped pretty quickly. Larger conversations are usually based around some kind of confusion. And we have a idiomatic consistency to the code to generally fall back on.

My issue has often been people not taking some concerns I have seriously, so again that makes me look like the nitpicker. On the other hand, I really feel little agitation when I'm getting nitpicked, usually because the reviewer has at least some point, or if not they are a junior dev who is confused about something. But I think my workplace has a good culture about these things in general and so I rarely feel bothered by reviews.

In case your team doesn't already do this, pick a style guide and automated style enforcer and have everyone use them automatically in their editors or when they commit. For example, black, isort, and flake8 for Python. If everyone's code is automatically made more stylistically similar, it cuts down on superficial stuff for people to bicker about during code review.

I also think about the Paul Graham maxim "keep your identity small" to avoid taking critiques of my code personally. I try to avoid identifying with my code too much and just see it as problem-solving.

Related blog post: https://mtlynch.io/human-code-reviews-1/

I thinks it's good to have a more general discussion in the team on how to handle the more opinion-based comments.

Are we talking about leaving written comments or F2F meetings? Because I've found the latter to be much more civil.

At some point, you have to directly address the issue with the reviewer or your/their manager. Some engineers waste time nitpicking at CR, don't stand for that shit. On the flip side, some people write shit code and need the mentoring, so the right move depends heavily on to what extent each possibility sounds like your situation. I eventually had to refuse to work with one guy over his wasting time code golfing shit at CR.

When I leave the nitpicky comments about doing things in a way that feels more correct to me, it's out of a felt obligation to make it look like I really read the diff. Sometimes it's even to excuse for me not understanding the broader context about why the change actually was made, so pointing out some language level change assuages the guilt of a half-assed job.

So then I don't take such comments to heart when I get them. On the system we use there's a "ignore" button for every comment raised. The commenter can reopen it, but it mostly doesn't happen, so I take it to mean they didn't care that strongly about it.

But maybe you feel a stronger sense of ownership over the stuff you write than I do. To me it's a cog in a larger system. I get paid whether the code goes in as I wrote it originally or if I make the requested changes. Ironically I suspect I'd feel much stronger about it if it was low-stakes unpaid open source project I was writing.

Personally, I prefer a system where you mark your PR comment with tag that explicitly calls out how important you think the issue is. It allows you to be complete, but also convey how much you care. I use:

MUST - I think this is an error of logic, not meeting requirements, or otherwise a major concern. I will fail the PR over this unless you convince me otherwise.

SHOULD - Probably should change this to make it more clear or adhere to standards. But if you feel strongly that your way is correct, I'm not really going to fuss.

MAYBE - random ideas, nitpicks, etc. Stuff that you can adopt or not at your leisure, and I don't really care at all.

This is a great point. We don’t have any kind of standard for that, and I think having my colleagues categorize their comments and forcing them to think about if it’s really a blocker would be helpful.

Cheers.