5 ms·
"Open source has always worked on a system of trust and verify" Not sure about the trust part. Ideally, you can evaluate the change on its own. In my experien
by adeebshihadeh 7mo ago
"Open source has always worked on a system of trust and verify"
Not sure about the trust part. Ideally, you can evaluate the change on its own.
In my experience, I immediately know whether I want to close or merge a PR within a few seconds, and the hard part is writing the response to close it such that they don't come back again with the same stuff.
(I review a lot of PRs for openpilot - https://github.com/commaai/openpilot https://github.com/commaai/openpilot)
- rafram 7mo ago[flagged]
- latency-guy2 7mo agoWhat kind of things would you like to hear? The default is you hear nothing. Most black boxes work this way. And you similarly have no say in the matter.
- BowBun 7mo agoWhy? I don't appreciate comments that cast doubt on decent technical contributors without any substance to back it up. It's a cheap shot from anonymity.
- deleted 7mo ago[deleted]
- 8n4vidtmkvmk 7mo agoI'm not the parent but if you know you want to merge a PR "within a few seconds" then you're likely to be merging in bad changes. If you had left it at know you want to reject a PR within a few seconds, that'd be fine. Although with safety critical systems I'd probably want each contributor to have some experience in the field too.
- colinmcdermott 7mo agoSounds like you misunderstood. They didn't say they are merging PRs after a few seconds. Just that the difference between a good one and a bad is often obvious after a few seconds. Edit: typos
- adeebshihadeh 7mo agoExactly, every PR starts with: 1. What’s the goal of this PR and how does it further our project’s goals? 2. Is this vaguely the correct implementation? Evaluating those two takes a few seconds. Beyond that, yes it takes a while to review and merge even a few line diff.
- stavros 7mo agoI'm not sure there are many ways to interpret "I know whether I want to merge a PR within a few seconds".
- jeremyjh 7mo agoYet I also agree with GP.
- theshrike79 7mo ago"*WANT* to close or *WANT* to merge". Not WILL close or WILL merge. You look at the PR and you know just by looking at it for a few seconds if it looks off or not. Looks off -> "Want to close" Write a polite response and close the issue. Doesn't look off -> "Want to merge" If we want to merge it, then of course you look at it more closely. Or label it and move on with the triage.
- ngcazz 7mo agoWhen there's time, you review, when there isn't you trust...
- 999900000999 7mo agoThat's the issue here. Even if I trust you, I still need to review your work before merging it. Good people still make mistakes.
- stavros 7mo agoWhat is the definition of trust if you still have to verify? How does "trust" differ from "untrust" in that scenario?
- johnnyanmac 7mo agotrust resudes the verification I suppose. Getting a PR from a trusted contributor would probably have me do a quick scan for obvious mistakes. And they'd know to keep the PR's small and on the right branch to help facilitate a scan. a new person with a big idea on the slightly wrong (but reasonable) channel would have more work in verification.
- adeebshihadeh 7mo agoWhat's the rush? Building good things takes time.
- johnnyanmac 7mo agodeadlines, money, attention. The usual things in industry.
- jgauth 7mo agoCool to see you here on HN! I just discovered the openpilot repository a few days ago and am having a great time digging through the codebase to learn how it all works. Msgq/cereal, Params, visionipc, the whole log message system in general. Some very interesting stuff in there.