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

Reinventing the pull request

lubeno.dev

Post reply on HN
Print view View original

81–90 of 96 posts

Re: Reinventing the pull request

#81

Post by l72 »

bewuethr wrote: post #61
esafak wrote: post #46

Earlier quoted context omitted.

link?

https://github.com/github/roadmap/issues/1218 is the tracking issue for it

I just want the simple feature of PRs updating when the target branch changes. For example, say I have two tickets: T-100 and T-101. Both are targeting main, but T-101 builds on top of T-100. I put up a PR for the T-100 branch against main, and put up a PR for the T-101 branch against main.

The T-101 PR can't really be reviewed yet, since you are looking at changes from both T-100 and T-101 (because T-101 was based on T-100).

Ideally, after T-100 is reviewed and merged, the T-101 PR would automatically update to show only the T-101 changes. But it doesn't. You have to manually rebase or merge main and push changes to the branch to get it to update. It would be great if GitHub handled this automatically.

Re: Reinventing the pull request

#82

Post by demorro »

MyHonestOpinon wrote: post #47

Regarding PRs. I have a an opinion which has become very unpopular since the rise of github. The way github does PRs is great for open source projects where you have submissions from a very diverse set of developers. You need to evaluate the whole PR as whole and accept it or reject it. But it is not ideal where you have a small team working on single product. The PR review becomes a gate and it has been my experienc…

> you spend a LOT of time trying to get your PR reviewed.

I've fixed this issue in several teams I've joined. Maybe some people won't understand this, as I didn't until I observed it multiple times over.

There are teams out there, perhaps the majority, that simply leave the decision as to when a PR will be reviewed nebulous, and at the discretion of team members. There is no formal obligation to do them in a timely manner, and there are no consequences if they are not done.

The solve to this is obvious and easy:

- Automatically assign specific people to PRs. No general team assignment. The submitter can add specific people in addition to PR's if they need domain experts, but the normal case is random assignment.

- Require PR's to be done within 24 working hours. If you cannot do this for whatever reason, you must communicate this to the team.

- There are consequences if you violate this policy.

The last one is the hard part for cowardly teams and cowardly managers. You do have to stay on top of it initially, and even when people have accepted this and gotten used to it, you can't forget about it because people will drift.

This isn't to say I'm against the direct integration model you propose, I can also see the appeal, it's just that problems with PR flows are mostly about cowardly management, not anything much to do with the actual process.

Re: Reinventing the pull request

#83

Post by [deleted] »

smcameron wrote: post #72
MyHonestOpinon wrote: post #47

Regarding PRs. I have a an opinion which has become very unpopular since the rise of github. The way github does PRs is great for open source projects where you have submissions from a very diverse set of developers. You need to evaluate the whole PR as whole and accept it or reject it. But it is not ideal where you have a small team working on single product. The PR review becomes a gate and it has been my experienc…

PRs can contain multiple commits. You need something like stgit to make it easy to make a bunch of small commits that appear to be the work of an omniscient genius who knew exactly what they were doing. Try using stgit for awhile, and you'll wonder how you ever lived without it.

[deleted]

Re: Reinventing the pull request

#84

Post by pavel_lishin »

demorro wrote: post #82
MyHonestOpinon wrote: post #47

Regarding PRs. I have a an opinion which has become very unpopular since the rise of github. The way github does PRs is great for open source projects where you have submissions from a very diverse set of developers. You need to evaluate the whole PR as whole and accept it or reject it. But it is not ideal where you have a small team working on single product. The PR review becomes a gate and it has been my experienc…

> you spend a LOT of time trying to get your PR reviewed. I've fixed this issue in several teams I've joined. Maybe some people won't understand this, as I didn't until I observed it multiple times over. There are teams out there, perhaps the majority, that simply leave the decision as to when a PR will be reviewed nebulous, and at the discretion of team members. There is no formal obligation to do them in a timely m…

At a previous job, it was the on-call engineer's duty to review PRs, as one of their responsibilities. A PR is just another interrupt, right?

We also paired this with giving the on call engineer near total freedom with what to do during their shifts, similar to 20% time (which was about the same percentage; 5 team members, weekly on call rotations.) They chose which tickets to pick up from the backlog, which also helped keep up with maintenance and taking care of bugs and issues that otherwise wouldn't get prioritized.

Re: Reinventing the pull request

#85

Post by demorro »

This whole thread reads like skill issues, sorry to say. This presumption that you have to stop work whilst you wait on a PR to be reviewed is absurd. Rebasing is not this impossible challenge and if it's stopping you making progress you need to grow up.

Re: Reinventing the pull request

#86

Post by demorro »

jtbetz22 wrote: post #74

> So even when you have a nicely structured commit history, you end up realizing that some things need to be changed and start appending a bunch of "fix" and "actual fix" commits at the end. I have found that this no longer needs to be an issue with agentic coding tools. Once I am happy with the end state of a branch, I tell Claude to rebuild the change from scratch as a set of atomic incremental commits. It adds abo…

It takes barely longer than that to do manually, the extra time really just being the typing out of the commit messages. Folks should have been doing this all along and it still shocks me to discover that they haven't been. However, I don't immediately dislike the idea of people using agents to do it, even though I'm sure it will occasionally introduce some strange choices and not fully capture the intention behind any given choice.

Re: Reinventing the pull request

#87

Post by fl0ki »

eviks wrote: post #79
fl0ki wrote: post #9

Earlier quoted context omitted.

> I think it would be great to have the ability to easily reorder/modify commits while in active development Take a look at `git rebase --interactive`.

That's not "easily". Easily would be: you drag your commit(s) from one place to another or copy/paste to achieve the same

If that's the kind of UX you prefer, please consider filing a feature request against your git UI of choice. My point is that git itself already has the core capability, and how convenient it is to use usually depends on your editor. (e.g. in vim, dd to cut a line and p to paste it in a new position is a very quick way to reorder)

Re: Reinventing the pull request

#88

Post by justinclift »

IshKebab wrote: post #29
jmalicki wrote: post #26

Earlier quoted context omitted.

You can manually stack PRs by making the merge target another branch. The workflow just makes it a pain, since you have to manually rebase in both the branch and the UI after the original PR merges.

That doesn't work if the base and PR branch are in different repos, which is the most common way of doing things in Github.

When the base and PR branch are in different repos, it'll still work if the PR branch's repo is a fork of the base one.

Re: Reinventing the pull request

#89

Post by IshKebab »

justinclift wrote: post #88
IshKebab wrote: post #29

Earlier quoted context omitted.

That doesn't work if the base and PR branch are in different repos, which is the most common way of doing things in Github.

When the base and PR branch are in different repos, it'll still work if the PR branch's repo is a fork of the base one.

No it doesn't. Here's the situation. You have an upstream repo github.com/fancyproject/fancyproject. You fork it to github.com/yourname/fancyproject. You make a branch `feature_0`, push it to yourname/fancyproject. Make a PR.

Next you create another branch `feature_1`, based on `feature_0`. You push `feature_1` to yourname/fancyproject. There is now no way to make a PR for `feature_1` that includes only the changes from feature_0 to feature_1. You can't do it because when you create the PR (in fancyproject/fancyproject) you can't select `feature_0` as a base branch because that branch doesn't exist in fancyproject/fancyproject.

Re: Reinventing the pull request

#90

Post by eviks »

fl0ki wrote: post #87
eviks wrote: post #79

Earlier quoted context omitted.

That's not "easily". Easily would be: you drag your commit(s) from one place to another or copy/paste to achieve the same

If that's the kind of UX you prefer, please consider filing a feature request against your git UI of choice. My point is that git itself already has the core capability, and how convenient it is to use usually depends on your editor. (e.g. in vim, dd to cut a line and p to paste it in a new position is a very quick way to reorder)

And my point is that all this 'core capability' stuff is not relevant to the discussion of good UI, similarly the fact that GitHub has Pull Requests doesn't help when it's bad UI that needs "stack" reinventing.

Case in point:

> dd to cut a line and p to paste it in a new position is a very quick way to reorder)

It isn't quick, you're just swiping the whole issue under the rug - first, you need the whole separate interface, but more importantly, this new interface is very primitive, you see close to no context, only some commit names, so it's not quick to find what to move and where because the content for those decisions is in a different place. Sure, you could add some vim plugin that expands it and adds per-commit info (what, you want to view the diff for all 3 commits you selected and DDed? Tough luck, you don't see the lines anymore! And even if you did, that's not this plugin), but then it's not your `--interactive` git "core" that does convenience

Post reply on HN