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

Reorient GitHub pull requests around changesets

mitchellh.com

Post reply on HN
Print view View original

181–190 of 210 posts

Re: Reorient GitHub pull requests around changesets

#181

Post by fahhem »

vanous wrote: post #123
fahhem wrote: post #82

Earlier quoted context omitted.

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!

I understand that you have to sell, but could you please do it somewhere else or take some decency and not to have your team to plug on every other comment here? Thank you!

We're actually a small company and only 2 of us posted here, most of the other plugs of us are by users. piotrkaminski is the only other user here from the company

Re: Reorient GitHub pull requests around changesets

#182

Post by ori_b »

da39a3ee wrote: post #133
u801e wrote: post #130

Earlier quoted context omitted.

> It'd be a useful thing to import without having to bring in the whole charade of using email and mailing lists (which most mail clients tend to be very unfavorable of in general nowadays) This is a misconception. git itself has commands (git-format-patch and git-send-email) that automate the creation of patches and sending changeset email threads to the mailing list. The only thing one needs to do is set the approp…

No, it is not a misconception. How do I apply a patch from gmail to a specific git repo on my computer? (I would genuinely like to know the answer, but it must not involve mutt, gnus, or dovecot!)

Many local mail clients also just let you pipe the message into a command; for example, in mutt, you could just type:

   |git am
once you have the message open.

Re: Reorient GitHub pull requests around changesets

#183

Post by jen20 »

ori_b wrote: post #182
da39a3ee wrote: post #133

Earlier quoted context omitted.

No, it is not a misconception. How do I apply a patch from gmail to a specific git repo on my computer? (I would genuinely like to know the answer, but it must not involve mutt, gnus, or dovecot!)

Many local mail clients also just let you pipe the message into a command; for example, in mutt, you could just type: |git am once you have the message open.

You may have a skewed idea of “most” here.

Most people are using gmail, but when it comes to local clients, Outlook, Apple Mail and Thunderbird are by far the most common and as far as I know none of them support this without extra steps.

Re: Reorient GitHub pull requests around changesets

#184

Post by 0xblinq »

0xblinq wrote: post #172
mikemcquaid wrote: post #136

Earlier quoted context omitted.

I left GitHub earlier this year after a decade. I’ve seen mockups, hack week projects and proof of concepts of this for the last 5 years (at least). A lot of engineers there knew this is the future that PRs need but GitHub at this point seems organisationally incapable of delivering these sorts of large improvements (Microsoft is perhaps partly but definitely not wholly to blame for this). Instead, they are midway th…

> Instead, they are midway through porting Rails views to use React, keeping most pages looking identical while introducing bugs and regressing previous usability improvements on a weekly basis. A real shame I predicted this the moment I saw the React dev tools icon going blue when browsing GitHub. My comment (which I can’t find right now, I’m on my phone) was along the lines of them going the “Reddit way”. A totally…

Replying to myself because I can't edit: Found my comment about this from 10 months ago: https://news.ycombinator.com/item?id=33583737

Not that this has any merit, as it could be seen from far away by anyone with > 5 years experience in this field.

And this just will just get worse, specially for end users. Let's see where we're in 10 months from now.

Re: Reorient GitHub pull requests around changesets

#185

Post by jen20 »

mikemcquaid wrote: post #136
Vendan wrote: post #127

Github had this planned in their old roadmap... But then they deleted it... https://web.archive.org/web/20220831234107/https://github.co...

I left GitHub earlier this year after a decade. I’ve seen mockups, hack week projects and proof of concepts of this for the last 5 years (at least). A lot of engineers there knew this is the future that PRs need but GitHub at this point seems organisationally incapable of delivering these sorts of large improvements (Microsoft is perhaps partly but definitely not wholly to blame for this). Instead, they are midway th…

This would explain something else too, probably: a few years ago I did a call with some GH folks talking about the idea of making the commit message applied to a squash merge part of the review itself.

Apparently this was very common feedback and I know that at least five other people who were maintaining large scale open source at the time gave it that week too. It’s never gone anywhere though, and as a result I have to disable all workflows except “rebase and merge” for every repo…

Re: Reorient GitHub pull requests around changesets

#186

Post by jen20 »

mmcnl wrote: post #171
oldtownroad wrote: post #10

I agree. As a way to minimise the pain on GitHub today, we disallow force pushing and enforce squash merging. Force pushing is a nightmarish behaviour, once a Pull Request is opened the branch must be append only.

That's a better solution than changing the Git(Hub) model. If this works for you, then why the need to change anything?

Because squash merging results in horrifically poor commit messages that are use to no one in future, because GitHub can’t be bothered to implement anything beyond a simple textarea for editing them.

Re: Reorient GitHub pull requests around changesets

#187

Post by jen20 »

akoboldfrying wrote: post #107
oldtownroad wrote: post #10

I agree. As a way to minimise the pain on GitHub today, we disallow force pushing and enforce squash merging. Force pushing is a nightmarish behaviour, once a Pull Request is opened the branch must be append only.

I don't understand the appeal of squash commits. AFAICT, the only advantage of squash commits is a nice "linear history", but you can get that from regular old merge commits just by adding --first-parent to your `git log` command -- and you still have that additional detail available on the second-parent commit chain if you need it. As soon as you start squashing commits, you throw away a very useful guarantee git gi…

The point is it enables bisect workflows that don’t suck. If half the commits in a tree are “fix misc derp” or “test passes now”, that workflow isn’t possible.

The short term and long term view of the work should be different.

Re: Reorient GitHub pull requests around changesets

#188

Post by jen20 »

ravenstine wrote: post #76
mhh__ wrote: post #58

Earlier quoted context omitted.

That's like saying it's OK to expect (say) Ford to make a car with no steering wheel. GitHub basically define the baseline for the entire industry and have millions upon millions to blow on trying to do better. (OK, Saab did actually do that once but they're weird)

The two aren't even remotely comparable. Whose compny went out of control and crashed because of GitHub missing a feature with their code review?

Probably too old to be GitHub, but Knight Capital comes to mind.

Re: Reorient GitHub pull requests around changesets

#189

Post by da39a3ee »

XorNot wrote: post #174
da39a3ee wrote: post #134

Earlier quoted context omitted.

I might be wrong but it sounds like you don't know about the GitHub PR feature where you select the lines, and GitHub allows you to edit them, and posts a diff when you're done, and if the author accepts it out becomes a jointly authored commit.

In situations where I'm doing PR reviews seriously in a multi-developer environment I'm usually on Gitlab. Gitlab definitely has some features for this sort of thing, but I've never been able to use them seamlessly or quickly. I was at one point considering writing a tool which would checkout an MR, then let you just edit it as per normal, and then would submit the whole thing back to a Gitlab MR as a set of proposed…

One way to do that is to open a second PR which targets the branch of the first PR. I think that's also probably the most natural way -- would you agree? I don't see it done much, but I've done it once or twice and I'd embrace it if it were more culturally standard.

Re: Reorient GitHub pull requests around changesets

#190

Post by u801e »

akoboldfrying wrote: post #149
u801e wrote: post #131

Earlier quoted context omitted.

> It's surprising to me that GitHub currently doesn't attach review comments to specific commits It is possible to comment on commits in github (you can do it by clicking on the sha1 of the commit and then making a comment on a line in the diff). But the comment won't show up in the main PR diff window. > But I don't understand how a contributor can feel pressure to address all of the reviewers' comments in a single…

>A changeset consists of one or more commits where each makes one logical change I don't yet see how that is different from just... a sequence of commits, which you can do now. (If you want to claim that you could quickly make a bunch of messy local commits and afterwards reorganise them into a more logical group of commits in a changeset -- you can already do that, without any new concept of changesets, by using `gi…

> I don't yet see how that is different from just... a sequence of commits

The difference is a sequence of commits that implement a feature where each commit basically is a logical change that does a single thing (e.g., add a function and associated test, add calls to a new function, etc) versus a sequence of commits with additional commits that fix issues brought up during review. Those additional commits really should be amended to the corresponding original commit rather than being a completely separate commit.

Another way to look at it is submitting an assignment where you have one page per problem solution as opposed to submitting that and then several more pages at the end containing fixes to those problem solutions rather than incorporating those fixes in the original set of pages.

Post reply on HN