| ▲ | adeebshihadeh 5 hours 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) |
|
| ▲ | jgauth 3 hours ago | parent | next [-] |
| Cool 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. |
|
| ▲ | ngcazz 5 hours ago | parent | prev | next [-] |
| When there's time, you review, when there isn't you trust... |
| |
| ▲ | 999900000999 4 hours ago | parent | next [-] | | That'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 3 hours ago | parent [-] | | What is the definition of trust if you still have to verify? How does "trust" differ from "untrust" in that scenario? |
| |
| ▲ | adeebshihadeh 3 hours ago | parent | prev [-] | | What's the rush? Building good things takes time. |
|
|
| ▲ | rafram 5 hours ago | parent | prev [-] |
| [flagged] |
| |
| ▲ | BowBun 5 hours ago | parent | next [-] | | Why? 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. | | |
| ▲ | 5 hours ago | parent | next [-] | | [deleted] | |
| ▲ | 8n4vidtmkvmk 5 hours ago | parent | prev [-] | | I'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 4 hours ago | parent | next [-] | | Sounds 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 3 hours ago | parent | next [-] | | Exactly, 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 3 hours ago | parent | prev [-] | | I'm not sure there are many ways to interpret "I know whether I want to merge a PR within a few seconds". | | |
| |
| ▲ | theshrike79 2 hours ago | parent | prev [-] | | "*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. |
|
| |
| ▲ | latency-guy2 5 hours ago | parent | prev [-] | | What 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. |
|
