Reorient GitHub pull requests around changesets • Hacker News Board

Re: Reorient GitHub pull requests around changesets

#81

Post by scubbo » Sun, Oct 01, 2023, 1:26 AM UTC

epolanski wrote: post #62

Jm2c, but by far the best place I've ever worked (my current client) we don't do code reviews at all unless the author wants feedback. PRs are good ways to defend your code base from bad code, and they were born in open source where you literally have no clue who the contributor is, but years of experience left me convinced that I don't want such a system where there's constant need to overview each other's work. I w…

I respect the thoughtfulness with which you made your point, but completely disagree. As another commenter stated - Code Reviews are, for me, primarily a method for communication and knowledge dissemination, secondarily a means to get a second pair of eyes on my work to ensure it's legible and that I didn't miss anything, and only as a distant third a means to "protect" the codebase.

Re: Reorient GitHub pull requests around changesets

#82

Post by fahhem » Sun, Oct 01, 2023, 1:27 AM UTC

ghthor wrote: post #44

I’m in 100% agreement. It’s difficult handling reviews of juniors in the current model, as you have to address the fir usage first and how to avoid these type of issues ITA BeFORE getting to the review at hand. Sign me up for change sets!

Then sign up for Reviewable, which supports this (we called them revisions) http://blog.reviewable.io/tracking-changes-in-a-code-review

Sorry for the shameless plug!

Re: Reorient GitHub pull requests around changesets

#83

Post by RHSeeger » Sun, Oct 01, 2023, 1:28 AM UTC

epolanski wrote: post #62

Jm2c, but by far the best place I've ever worked (my current client) we don't do code reviews at all unless the author wants feedback. PRs are good ways to defend your code base from bad code, and they were born in open source where you literally have no clue who the contributor is, but years of experience left me convinced that I don't want such a system where there's constant need to overview each other's work. I w…

I trust and respect the people I work with. I also prefer to have code reviews, even (especially) of my own code. Code reviews can point out code that

- Doesn't consider an edge case (NPEs being the most common)

- Doesn't match up well with the "standard" way we do things (and, all things being equal, using the same approach in every place improves maintainability)

- Has a simpler approach

- Doesn't have a comment where it's not clear _why_ the code is doing something (that the author of the code sees as obvious, because they wrote it)

- Has misleading method/class/field names

These are all things that I've seen from both myself and others on my team. Having one other team member take a look at my code before it goes to QA can save a lot of time later.

Re: Reorient GitHub pull requests around changesets

#84

Post by scubbo » Sun, Oct 01, 2023, 1:29 AM UTC

da39a3ee wrote: post #7

> I'm sure I'm wrong about some detail about some of the points above. Someone is likely to say "he could've just done this to solve problem 5(a)" Yep, I'm going to do that! > Work-in-progress commits towards addressing review feedback become visible as soon as the branch is pushed. This forces contributors to address all feedback in a single commit, or for reviewers to deal with partially-addressed feedback. This po…

Yeah, that one baffled me, too. The rest makes sense, though!

Re: Reorient GitHub pull requests around changesets

#85

Post by jacobegold » Sun, Oct 01, 2023, 1:30 AM UTC

GitHub actually stores all the data necessary to do this without changing your ref model as the article suggests — even if you force push, it never garbage collects the commits that the branch used to point to.

I work on a tool that includes a UI for diffing versions of a GitHub PR, but you can totally get the same thing via messing with GitHub.com URLs.

Re: Reorient GitHub pull requests around changesets

#86

Post by fahhem » Sun, Oct 01, 2023, 1:31 AM UTC

0x073 wrote: post #79

piotrkaminski wrote: post #29

Earlier quoted context omitted.

Here's how I like to think about it: GitHub is a generalist. They have a big platform with lots of features besides code review, so even though they also have lots of employees they won't be able to focus on code review as much as a dedicated company could. They also have a huge number of users to please so they can't afford to rock the boat too much or make the learning curve too steep. I think therefore it's pretty…

What's the reason as a company to pick GitHub? Pull request and everything around it are by far the most important part. If i must pay for GitHub and an external tool, isn't GitHub just an dumb overpriced git storage.

GitHub is a platform with dozens of tools, I think of it as a basic toolbox. It's great, but if you're hammering nails all day, you should invest in a nailgun. Doesn't mean you don't use the hammer or the wrench in your toolbox, but when you care about one task a lot, you invest in the tool to do that task better

Re: Reorient GitHub pull requests around changesets

#87

Post by jacobegold » Sun, Oct 01, 2023, 1:32 AM UTC

scubbo wrote: post #81

epolanski wrote: post #62

Jm2c, but by far the best place I've ever worked (my current client) we don't do code reviews at all unless the author wants feedback. PRs are good ways to defend your code base from bad code, and they were born in open source where you literally have no clue who the contributor is, but years of experience left me convinced that I don't want such a system where there's constant need to overview each other's work. I w…

I respect the thoughtfulness with which you made your point, but completely disagree. As another commenter stated - Code Reviews are, for me, primarily a method for communication and knowledge dissemination, secondarily a means to get a second pair of eyes on my work to ensure it's legible and that I didn't miss anything, and only as a distant third a means to "protect" the codebase.

Not to mention the value of a diff/pull request/changelist/patch/merge request (so many names...) and its corresponding discussion as a historical artifact for your team to reference later on!

Re: Reorient GitHub pull requests around changesets

#88

Post by fahhem » Sun, Oct 01, 2023, 1:34 AM UTC

jacobegold wrote: post #85

GitHub actually stores all the data necessary to do this without changing your ref model as the article suggests — even if you force push, it never garbage collects the commits that the branch used to point to. I work on a tool that includes a UI for diffing versions of a GitHub PR, but you can totally get the same thing via messing with GitHub.com URLs.

I agree, we work on Reviewable, which uses those same blobs.

However, we add tags to commits we're referencing, just in case someone at GitHub gets around to implementing garbage collection!

Re: Reorient GitHub pull requests around changesets

#89

Post by llimllib » Sun, Oct 01, 2023, 1:34 AM UTC

kemayo wrote: post #40

I work with Gerrit in my job, and find a stack of patches to be a useful way to deal with things... but I've also seen that it definitely has a learning curve for people who're not used to it. There's something to be said for the GitHub pull-request "just smush together all the commits on this branch" model in terms of ease of understanding. It's possible that better tooling would help there, of course. (A surprising…

At my previous job the cofounder introduced gerrit; I loved the model but it became immediately apparent that it was too complex for our team and we abandoned it after I spent a ton of time doing tech support for teammates.

Re: Reorient GitHub pull requests around changesets

#90

Post by jacobegold » Sun, Oct 01, 2023, 1:36 AM UTC

0x073 wrote: post #79

piotrkaminski wrote: post #29

Earlier quoted context omitted.

Here's how I like to think about it: GitHub is a generalist. They have a big platform with lots of features besides code review, so even though they also have lots of employees they won't be able to focus on code review as much as a dedicated company could. They also have a huge number of users to please so they can't afford to rock the boat too much or make the learning curve too steep. I think therefore it's pretty…

What's the reason as a company to pick GitHub? Pull request and everything around it are by far the most important part. If i must pay for GitHub and an external tool, isn't GitHub just an dumb overpriced git storage.

How you do PRs is definitely important, and should be part of a company's consideration here, but remember that PRs are already a layer of abstraction over the SCM. Self-hosting Git is definitely not as easy as setting up a GH account.

Not to mention that you get an easy way to spin up your CI/CD workflows in GH Actions (which of course definitely has its own problems, which there was another popular HN post about recently). There's a reason why it's the default for new companies -- if there was something much better, it wouldn't have the market share it does I think. Familiarity coming from OSS is also important.