this post was submitted on 28 Feb 2024
1466 points (99.7% liked)

Programmer Humor

32415 readers
817 users here now

Post funny things about programming here! (Or just rant about your favourite programming language.)

Rules:

founded 5 years ago
MODERATORS
 
top 50 comments
sorted by: hot top controversial new old
[–] [email protected] -2 points 8 months ago

Or! Alternatively! Bring your wife to the situation and have her talk about her dreams.

[–] [email protected] 16 points 8 months ago* (last edited 8 months ago) (1 children)

I would be fine with it if my job was just that review stuff.

But man when you are in the middle of something and you have to review something because they need it soon... It's annoying as fuck.

[–] [email protected] 16 points 8 months ago

Reviewing code is part of your job. But you also deserve uninterrupted focus time. Just block focus time blocks in your calendar and check if your peers need reviees 2-3 times a day.

[–] [email protected] 2 points 8 months ago

God damn it.

[–] [email protected] 8 points 8 months ago

Certain this works 100% in the wild. Main issue will be trying to SSH into the server, unless you can borrow their hotspot.

[–] [email protected] 4 points 8 months ago

I promise I’m not purposefully ignoring you I just forgot and could use a nudge

[–] [email protected] 14 points 8 months ago

As a required reviewer on a lot of things, I envy that bear so, so much.

[–] [email protected] 11 points 8 months ago (1 children)

don't review just blind accept and merge it's more fun that way

[–] [email protected] 4 points 8 months ago
[–] [email protected] 7 points 8 months ago

No! Ask him to leave a rating on your encounter with him.

[–] [email protected] 9 points 8 months ago (2 children)

I haven't done any serious programming in a long time. Is this mostly about corporate process and hierarchies for programming or does this apply to open source projects as well?

Seems really demoralizing putting in the work to add something to an open source project and having it waste away unreviewed and unappreciated.

[–] [email protected] 4 points 8 months ago

Applies to both.

[–] [email protected] 16 points 8 months ago* (last edited 8 months ago)

It's more about scale. Small open source projects might get one PR a month. Your average tech company is dealing with dozens of PR every single day. Review fatigue is real in these environments

[–] [email protected] 1 points 8 months ago

To be fair, we sometimes have to look through multiple related documentation and tickets to make sure the change was actually reviewed and approved by the necessary teams (network, security, etc.). We also have an SLA for PR reviews/approvals and some people have a habit of sending it out for approval at the last minute of the change window.

[–] [email protected] 97 points 8 months ago (3 children)

We decided that everyone in the team is allowed to approve changes. If no one has reviewed your change within 24 hours you are allowed to approve it yourself. It will usually come up in the daily sync that a self approval is imminent, which usually leads to someone taking a look.

[–] [email protected] 2 points 8 months ago (1 children)

which usually leads to someone taking a look

Nevermind the idea that one reviewer is somehow sufficient, this sounds like pure fantasy. Did you forget a "/s"?

[–] [email protected] 9 points 8 months ago (1 children)

Who said anything about only requiring 1 reviewer? And no, I did not drop an /s. You should try working for a healthy team where everyone takes collective responsibility and where the teams progress is more important than any one person's progress.

[–] [email protected] 2 points 8 months ago

I get the feeling you feel like I was somehow calling you out. I want to clarify the the intent of my message was more in the spirit of "wow must be nice" than "you're making that up". But also I'm just interested in how different your experience is from mine.

Who said anything about only requiring 1 reviewer?

I must have misunderstood. You said "If no one has reviewed your change within 24 hours you are allowed to approve it yourself." To me, that sounds like, after 24 hours of no review, one self-approval is considered sufficient. That, in turn, seems to imply that before 24 hours, one non-self-approval is probably sufficient, no?

You should try working for a healthy team where everyone takes collective responsibility and where the teams progress is more important than any one person's progress.

I've had team members in the past who are very self-focused, they tend to close a lot of tickets and look good, then get promoted out, leaving an unmaintainable mess behind. Allowing that is generally a failure of leadership. But right now, that's not our problem, and what you describe is pretty much how we operate.

I'd love to work on a team where everybody took code review a lot more seriously, believe me, it'd be nice, but my team does generally get everything approved, with at least two non-self approvals, in under 24 hours. If something is getting ignored because people are busy and it's a large change because we aren't perfect, and there is some reason to get it in soon, it just takes a quick request on Slack to get the needed attention.

What I found surprising about your description was more that the potential of a self-approval coming up would, in itself, get people's attention, rather than somebody reaching out personally and asking for a review.

Our big weakness is review quality, not quantity. It's crazy the number of times I look at something and see the two or three approvals already, start going through it, and find issue after issue. I see that on other teams as well, where there's usually only one or two devs who ever really make any comments on a review, it seems to be very common.

[–] [email protected] 16 points 8 months ago (2 children)

Self-approval leads to a road of sadness. For example, a theoretical company needs to self-renew an ssl cert. No problem, the cert will be stored with the rest of the secrets and retrieved in a secure way on deployment. Unfortunately if you don't store the cert key in a secure way, the deployment still works fine and you don't need to figure out the "onerous" encryption process.

So you push the private key to the company git repo, and then deploy the cert! Done and Done.

[–] [email protected] 0 points 8 months ago

If you don't establish an encryption mechanism for secrets that allows for automatic, in memory decryption on deployment from the start of your project, then your project is run by incompetent developers/ops specialists/architects/management/etc. and deserves to fail.

[–] [email protected] 7 points 8 months ago

We have well established ways to deal with secrets. Also, everyone is responsible enough to not self approve changes where they do things they are uncertain of.

[–] [email protected] 27 points 8 months ago (1 children)
[–] [email protected] 43 points 8 months ago (2 children)

Surely this will just devolve into "no reviews ever".

[–] [email protected] 5 points 8 months ago* (last edited 8 months ago)

We very seldom resort to self approvals. Everyone in the team see code reviews as important. But also that progress trumps code review.

[–] [email protected] 8 points 8 months ago

More like "Jerry reviews everything"

load more comments
view more: next ›