Hacker Newsnew | past | comments | ask | show | jobs | submit | dust-jacket's commentslogin

> PR approval is too boolean. The PR is approved or it's not approved. Real code review, like real life, lives in the middle

This is have-your-cake-and-eat-it. PR approval is a permission so is a boolean. Of course it is. Either the code can be merged or it can't.

What's being described really here is just something to make you feel slightly better about yourself whilst approving code you hate ("we should revisit this..."). Just open a new ticket.


Gerrit has -2...+2.

-2: This is a bad idea, don't do that

-1: This is a good idea but needs improvement

+1: LGTM but I don't have enough knowledge or authority to approve

+2: Approved


This seems like it’s conflating problems. It’s actually two different problems:

1. Is the PR suitable, and therefore should be approved, and

2. Is this person suitable to make that decision.

If 2 is false then the person should remove themselves from the list of reviewers. Then 1 can follow its normal process.


I was in camp 'boolean', but I think this has convinced me. I always had a problem that there were developers who didn't really understand the code, but would click 'approve' anyhow because they didn't see any problems in the parts they understood.

This meant that they were completely unable to actually 'approve' a review, but were only able to reject it. They were juniors, so they'd eventually get to that point, but by then, everyone would be used to just ignoring their approvals.

This provides that middle ground.


Either the code gets merged or it does not. That's the inherent boolean part.

Given that, what's wrong with simply commenting on the PR to document the concerns, issues, lack of knowledge, etc?

Unless you're using those +/-2 to achieve some sort of goal... but you can also do that with labels, tags, etc. on the PR.


everything except +2 is unapprove.

The nuance is comments on the PR itself, rather than the state of the approval, which is binary (or ternary, if you want to count leaving it in an unknown state for extended periods of time).


What if you want someone to look at a portion of it but they don't know enough to approve the whole thing. They give +1

Someone else knows the other portion well and sees the +1 and decides to +2.

In practice this ends the stalemate where partial owners don't feel confident to approve the whole thing


The PR needs to have someone who knows the whole thing.

Having several people review each separate parts but not understanding the others' can cause interaction bugs. If such bugs cannot happen (say, due to modularity, or type safety guarantees etc), then it won't be the case where you need to have a partial approve.

I am not a fan of partial approve. Either you think the code is approvable, or it isn't.


And Gerrit has multiple review label that can be customized[0].

So you could require `Verified+2` (CI), `Code-Review+2`, and `Design+2`, for example.

[0]: <https://gerrit-review.googlesource.com/Documentation/config-...>


I mean, that’s fair no? If the UX creates an impasse for the user then this leads to friction in the process. There’s more than one way to address it. One is the user overcomes his own internal dilemma, the other is the UX helps him get there. For example, would be cool if there was a way to do a conditional approval with an issue tied to a stacked PR or something similar (just throwing ideas, point is to surface up the friction as a UX take not a protocol or API issue with git)

This exists in Azure DevOps as Approve with Suggestions

But that wasn't your point. Your point was that, because Russia, it _didn't matter_ if it did constitute stealing.


No, you seem to have misunderstood my point.


interesting, I'd assumed the lowest tier of hetzner (4.50/m, 2 cpus, 4GB ram) wouldn't hold up to that.

must be very light, for so much traffic. any more details?


It's a BitTorrent tracker

tracker.mywaifu.best:6969/announce

Running https://github.com/ckcr4lyf/kiryuu

(Disclaimer: I'm the author of kiryuu)

CPX11, so 2vCPU/2GB


ah, thank you. Haven't worked in java for a bit now, but that was the only one I read where I was like "I'm sure we didn't have to avoid this when I worked on java".

The rest were all very familiar. Well, apart from the new stuff. I think most of my code was running in java 6...


I now really want my city to employ local artists to redraw all the street markings.

Chaos, sure, but beautiful chaos.


I really like the street sign analogy.

But in my case it was the other way around. I work in a Kowloon Walled City of code: dozens of intersecting communities with thousands of informally organized but largely content contributors. It looks like chaos, but it works ok.

Code formatting really did feel like a new neighbor declaring "you know what this place needs, better-marked bus lanes!" as though that would help them see the sky from the bottom of an ally or fix the underlying sanitation issues. As you might imagine, the efforts didn't get far and mostly annoyed people.

But as the GP said, it all depends on the culture. If you pick up and move to Singapore you'd damn well better keep your car washed and your code style black.


I mean fired and resigned when it became clear you'd be fired are the same thing really.

We're not actually entitled to know the exact details of someone's job ending. They worked there. Now they don't. That much is the bit we're entitled to.


For public misconduct like this, we should get to know if he was fired (or asked to resign) as opposed to his making the independent decision to find work elsewhere or retire or whatever. We should get to know if he left because the company wanted him gone or because he wanted to be gone.


Nah, The Register is far more strongly anti-AI. Mention AI and systemd in the same article and watch them froth


I absolutely disagree.

This has been done very professionally. They pulled the article. They handled the personnel matter. They didn't try to pretend it hasn't happened.

Why are people here acting like retracting an article is an attempt to hide something. They literally replaced the whole text with a note from the editor saying "this article was bad".


> Why are people here acting like retracting an article is an attempt to hide something.

Because the retraction notice hid the article name and the author name?


This is kinda cool. Thanks for sharing.


This is mad but cool. Keep at it.


Thanks, mad is fun for me! It costs me nothing if it fails.


Consider applying for YC's Summer 2026 batch! Applications are open till May 4

Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: