Hacker News Board Hacker News, browsed like a classic message board
Live data from Hacker News

If I could make my own GitHub

matduggan.com

Post reply on HN
Print view View original

121–130 of 160 posts

Re: If I could make my own GitHub

#121

Post by cpan22 »

dust-jacket wrote: post #47

> 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 thi…

Well I think the bigger issue here is that GitHub is too decoupled from an issue tracking system - its a lot of manual overhead to constantly have to keep in sync. Linear does an ok job but far from ideal

Re: If I could make my own GitHub

#122

Post by LinuxAmbulance »

shevy-java wrote: post #30

> I was prompted to write this after reading the good post about Ghostty leaving GitHub but it's something I've written and talked about for a few years. Many of us were annoyed already when Microslop, 'xcuse me, Microsoft assimilated GitHub. But we have to be realistic - alternatives often sucked. Sourceforge? I find creating issues there annoying to no ends. I can use gitlab, which is a bit better than sourceforge,…

The problem is running a SaaS product costs a lot of money.

Mega corps can fund that, but even large numbers of devs on small budgets don't have the money to do the same.

So any commercial project will inevitably trend towards supporting the interests of mega corps over the average person.

Re: If I could make my own GitHub

#123

Post by xmodem »

dust-jacket wrote: post #47

> 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 thi…

I just want a button that says "approve and merge these 3 commits now but these two need re-work"

Re: If I could make my own GitHub

#124

Post by sandeepkd »

ahartmetz wrote: post #58
dust-jacket wrote: post #47

> 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 thi…

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

I think we are normalizing the PR process here, in reality its more convoluted and a good reflection of the team/organization itself. The relationship between the author and reviewer can have negative impact on the rationale and desired outcome of the PR itself.

To run the process smoothly, one can just hope that the team/tech lead is an ideal developer. Otherwise they are in a position where no one senior than them is available for the code review and any one junior would just rubber stamp their PR's.

Re: If I could make my own GitHub

#125

Post by zephen »

rprwhite wrote: post #102
ahartmetz wrote: post #58

Earlier quoted context omitted.

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 like the idea of being able to merge a PR that is a partial solution, while keeping the issue open to reflect that it is only partially done. It kinda makes sense to do this in a single action.

Also:

> If [a person is not suitable to make the decision of whether the PR should be approved] then the person should remove themselves from the list of reviewers.

This doesn't reflect what sometimes happens in real life. Someone could have sufficient specialized knowledge to be able to veto a PR, without having sufficient broader knowledge to approve a PR. That person should definitely be left on the reviewer list, with the ability to veto, the necessity to remark if he has vetoed or not, and the inability to definitively approve.

It is necessary for this specialist to notate "I have finished examining this PR, and there is nothing within my expertise that would cause me to veto it" before the PR is advanced.

Unfortunately, in a binary system, that often equates to him having to say "I approve" even though this does not truly capture the intent. Then you wind up with hacky work-arounds, like requiring a minimum number of approvals.

Re: If I could make my own GitHub

#129

Post by BigTTYGothGF »

dust-jacket wrote: post #47

> 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 thi…

> Either the code can be merged or it can't.

Not an intuitionist, I see.

Re: If I could make my own GitHub

#130

Post by pdpi »

BigTTYGothGF wrote: post #129
dust-jacket wrote: post #47

> 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 thi…

> Either the code can be merged or it can't. Not an intuitionist, I see.

If you are an intuitionist, excluded middle isn't an axiom, but is still provable/assertable on a case-by-case basis. This is a scenario where asserting it is entirely reasonable.
Post reply on HN