6 ms·

Edit: This commit was written to the original submission: https://talks.golang.org/2015/state-of-go.slide#7 https://talks.golang.org/2015/state-of-go.slide#7 -

Edit: This commit was written to the original submission: https://talks.golang.org/2015/state-of-go.slide#7 https://talks.golang.org/2015/state-of-go.slide#7 -------------- It seems like these people simply don't understand Github very well. Can only view diffs on a single page (can be very slow). Cannot compare differences between patch sets. Accepting a patch creates a "merge commit" (ugly repo history). Don't use the merge button, just add the requester's repo as a remote to yours and use your familiar tools. If possible and done, a fast-forward merge + push will also close the PR. Comments are sent as they are written; you cannot "draft" comments. How is that different from pull requests via emails? (Also, on the website itself the comments can be edited.) To create a patch one must fork the repository publicly (weird and unnecessary). What exactly is weird about that? It makes it possible for the requestor to craft their changes with full control, without requiring upstream to give them write access. This point is just entirely nonsensical. In general, pull request culture is not about code review. A strong claim, but one without any justification, and which is, in my experience, as far from the truth as possible. Edit: In light of their complaints about the need to fork, i have to say that their current contribution process can in its entirety be described as weird, unnecessary and baroque: https://golang.org/doc/contribute.html https://golang.org/doc/contribute.html

kasey_junk 12y ago
> What exactly is weird about that? I have no idea what they specifically mean, but I can tell you, that the github requirement of pr to come from public repositories on github is something that bothers me occasionally. There are lots of reasons why I may not want my github fork to be public or I don't want to have a github fork at all but do want to contribute to a repo hosted there. This is a distributed version control system we are talking about after all. As for their claim about pull request culture, again I'm not sure what they specifically mean, but I find the github code review tools to be very rudimentary and suspect that lots of other people with experience with more sophisticated code review workflows feel the same way. Github is a great service for some things but it certainly is not centrally about code review.
- Mithaldu 12y ago
  Honest curiosity here. Given that there's nothing stopping you from deleting your fork after the PR is done, what reasons can you name for this: There are lots of reasons why I may not want my github fork to be public or I don't want to have a github fork at all
  - kasey_junk 12y ago
    The simplest reason is if what I was contributing was done on my employers time and they have strict IP restrictions. It is much, much easier to vet a single changeset than it is to a) verify a whole repo history and b) coordinate around my public/private github identities.
    - Mithaldu 12y ago
      I was hoping for more reasons, since the very first and simple one is usually the one that has the most and simplest workarounds. You could for example make a patch with git on your local machine and attach that to an issue.
      - kasey_junk 12y ago
        And then I have to follow a completely different work flow and inconvenience the maintainers.
    - lomnakkus 12y ago
      I'm not sure I understand. Vetting a changeset is basically the same thing as verifying the repo. Just look at the hash of repo A (your clone), check that it matches B (upstream), check that your repo A + "patch" matches pull request. What am I missing?
      - kasey_junk 12y ago
        Branches and the ref log.
  - 12y ago
justinmk 12y ago
> Don't use the merge button, just add the requester's repo as a remote to yours Actually, you don't need to do that. GitHub provides a special pulls remote "namespace" on the upstream repo, so you can add it as a fetch pattern to your .git/config like so: [remote "upstream"] url = https://github.com/neovim/neovim.git fetch = +refs/heads/*:refs/remotes/upstream/* fetch = +refs/pull/*/head:refs/pull/upstream/* Then when you `git fetch --all`, you will have ALL pull requests available in your local repo in the local pull/ namespace. To check out PR #42: git checkout -b foo refs/pull/upstream/42
- Mithaldu 12y ago
  That is great, and i thank you for bringing that to my, and everyone else's attention! :)
- jbish 12y ago
  Yes! I've been wanting to do this and didn't know it was this easy.
- ChuckMcM 12y ago
  Wow you are my new favorite person. What I was going to say was that the GP was correct in that it just took a bit more work on the maintainers part to keep the history clean, but even that is unnecessary apparently :-)
- hacknat 12y ago
  You rock!
- eiopa 12y ago
  Good tip. It's interesting that other systems has a nicer implementation for it. In Phabricator, it's: $ arc patch D12345 and it checks out revision 12345 into an appropriately named branch.
- zak_mc_kracken 12y ago
  This is... amazing. Thank you so much.
- 12y ago
CHY872 12y ago
Just out of curiosity, have you ever used a tool such as Gerrit or Atlassian Stash? There are different mindsets, and both have their advantages.
- Mithaldu 12y ago
  I must admit i have not, and to make clear: I am not taking offense to their implication that Github is worse at some things than other tools (it is). My post above is merely objecting to the opinions that were borne out of some lack of knowledge (which is acceptable), yet are posited as facts (which is not).
  - CHY872 12y ago
    Mm, I'm not sure. These are slides, presumably designed to go alongside a talk, which could have fleshed out the points better. If you consider them as being summaries of what a speaker's saying, it makes much more sense. Fwiw, of the six bullets, I see three which are clearly subjective, one which is a clear advantage of gerrit, and two which I don't really know enough about to refute. When it comes down to build systems, I think it's very easy to spend a lot of time moving sideways or backwards - and if the tool you're moving to has deficiencies compared with what you had before, it can be very frustrating. Certainly, if the tool people want you to move to offers few advantages over your current infrastructure, a quick dismissal is reasonable. Getting set up with their current contribution system might seem a little clunky, but I don't think it's particularly hard to do, and definitely seems like a 'run-once' thing. Once it's set up, it seems to integrate into a workflow well. Their contributors will have their current workflows set up nicely - and unless github's issues system has compelling advantages, it's definitely not worth them switching due to the temporary loss in productivity.
bdarnell 12y ago
>> Comments are sent as they are written; you cannot "draft" comments. > How is that different from pull requests via emails? (Also, on the website itself the comments can be edited.) With github PRs, notifications are sent as soon as you write your first comment; in systems like gerrit and rietveld (and email) they are not sent until the reviewer chooses to send them. This leads to either awkward interactions if you start replying to comments while the reviewer is still reviewing, or unresponsiveness if everyone introduces hysteresis to avoid this situation. This is IMHO the biggest problem with Github PRs; I don't necessarily agree with the Go team's decision to abandon the PR system but on smaller projects I have pushed for rietveld over PRs for this reason.
- cbr 12y ago
  My favorite are the comments like: 1:12pm line 33: Why are you doing this? 1:13pm line 33: I see, sorry, ignore my earlier comment. A system that lets you draft comments and then send them out in a batch can avoid a bunch of noise. On the other hand, though, people who aren't expecting it can get stuck with draft comments they don't send out.
  - amenghra 12y ago
    I think we have the technology to show that a piece of text is a draft and hasn't yet been sent. Imho, the end user should be able to choose between live commenting vs draft+send.
  - Gankro 12y ago
    Usually if this happens, it's a good hint that this piece of code could use a good comment. Not always, of course.
pjc50 12y ago
Don't use the merge button, just This is the smell of git plumbing again. Don't use the obvious UX that's been presented to you, do some other workflow that doesn't appear in the documentation.
- jrochkind1 12y ago
  Well, it depends on what documentation you mean. The alternative is using standard git commands that are in the git documentation, which is exactly what OP's alternative presumably involves? (But I am entirely sympathetic to the opinion that git itself exposes too much plumbing and has a pretty baroque end-user interface for doing certain things. And it's also certainly legit to wish or suggest that _github_'s UI worked differently than it does, although the merge commits don't really bother me, and some people prefer them, it's a point of some contention).
- xkarga00 12y ago
  But really... stop using the merge button.
- rdtsc 12y ago
  I see you've been downvoted and I think unfairly. I agree with you though. Looking back at arguments people make about how to "do it properly", it looks like something is broken. Git is broken, maybe Github is broken, documentation, UI, marketing. Something is though. When an obvious UI element is there, seemingly designed to do merges, and then everyone says "no, no, do this other thing", like send emails, then rebase here on top of that, make a ref pattern in your ~/.gitconfig ...
- lomnakkus 12y ago
  Git != github. (Does this really need to be said?)
  - rdtsc 12y ago
    Saddly it does. There is probably a significant number of developers who never used git without Github. As a funny aside, when Github breaks the thing to say is "I wish one day someone would invent a decentralized version control system" ;-)
deleted 12y ago
[deleted]
enneff 12y ago
Half your post is saying we should just use different tools, but we are. We're using Gerrit, that lets us work the way we want. On the contributor instructions: note that if you just want to use plain git (and not our git-codereview tool) then you can stop at "Register with Gerrit." We created the tool to provide a more familiar review process for the people that used our previous system. If we took your advice and used offline diff tools, etc, we would need a similar page explaining how to do all of that. None of this convenience comes without an up front cost.
- Mithaldu 12y ago
  Thanks for the direct answer. I think my stance wasn't clear enough, so i'll reword what i wrote in another comment here. My issues are two-fold: You could have just said on that slide: "We want a central code review system, so people do not have to learn Git. Github isn't terrible, but Gerrit is much better." Instead you ended up putting up a list of points that make github seem like some kind of fatally flawed thing, while frankly putting people off with inaccuracies/subjectivities. Secondly, by not allowing things by github you're forcing people to learn something else. Most developers experienced with Git will also be very familiar with Github. You're telling those people to instead go and learn something else. That will result in some people deciding it's not worth the trouble. I'm fully aware it's up to you to decide whether you're willing to pay that price, but personally i find it a bit odd that you can't simply do both. And as for, apparently, most of the documentation on your contribute page being safely ignorable: If that is truly the case, i recommend rewriting that to make it obvious, because right now it's anything but. :) If the last bit is the only good thing that comes out of this, then i'll be happy.
  - farresito 12y ago
    If they need to learn something new, they will. It's part of being a programmer. For me, github's way of doing things is way more flawed that gerrit.
zak_mc_kracken 12y ago
It's not exactly the first the time Go team is showing that they haven't spent much time researching current practices, theory, research and tools in the field of software engineering. Everything about the language screams of coming from minds who stopped learning new things in the late 90s.
- dsymonds 12y ago
  I think you mean s/researching/adopting/, since you are only referring to the outcome, not the study or decision making that arrived at the outcome. The Go team generally knows about modern things; we deliberately choose not to use them when we don't think they suit Go.
- themightykevdog 12y ago
  This is so colossally misguided a comment it's hard to know where to begin. So I won't.
  - zak_mc_kracken 12y ago
    See my comment about "not wanting to learn". Happy to engage in a discussion about modern software engineering. Are you?
thockingoog 12y ago
I'll concur that the GitHub code-review UX is atrocious. Having used a small number of other code review tools, I can't fathom how GitHub's tooling is so brain-damaged. It does a few cute and clever things, but I totally agree with this slide deck's gripes. Email and GitHub are not really compatible.
hueving 12y ago
These are all common issues with people that came from a different CVS and didn't learn what was different about git. They just learn enough to reach parity with the "everything is linearly developed" workflow. When you live with that assumption, things like merge commits and pull requests seem silly and overkill since you live in the "patch is developed/reviewed in isolation" world.

kasey_junk 12y ago

> What exactly is weird about that? I have no idea what they specifically mean, but I can tell you, that the github requirement of pr to come from public repositories on github is something that bothers me occasionally. There are lots of reasons why I may not want my github fork to be public or I don't want to have a github fork at all but do want to contribute to a repo hosted there. This is a distributed version control system we are talking about after all. As for their claim about pull request culture, again I'm not sure what they specifically mean, but I find the github code review tools to be very rudimentary and suspect that lots of other people with experience with more sophisticated code review workflows feel the same way. Github is a great service for some things but it certainly is not centrally about code review.

Mithaldu 12y ago
Honest curiosity here. Given that there's nothing stopping you from deleting your fork after the PR is done, what reasons can you name for this: There are lots of reasons why I may not want my github fork to be public or I don't want to have a github fork at all
- kasey_junk 12y ago
  The simplest reason is if what I was contributing was done on my employers time and they have strict IP restrictions. It is much, much easier to vet a single changeset than it is to a) verify a whole repo history and b) coordinate around my public/private github identities.
  - Mithaldu 12y ago
    I was hoping for more reasons, since the very first and simple one is usually the one that has the most and simplest workarounds. You could for example make a patch with git on your local machine and attach that to an issue.
    - kasey_junk 12y ago
      And then I have to follow a completely different work flow and inconvenience the maintainers.
  - lomnakkus 12y ago
    I'm not sure I understand. Vetting a changeset is basically the same thing as verifying the repo. Just look at the hash of repo A (your clone), check that it matches B (upstream), check that your repo A + "patch" matches pull request. What am I missing?
    - kasey_junk 12y ago
      Branches and the ref log.
- 12y ago

justinmk 12y ago

> Don't use the merge button, just add the requester's repo as a remote to yours Actually, you don't need to do that. GitHub provides a special pulls remote "namespace" on the upstream repo, so you can add it as a fetch pattern to your .git/config like so: [remote "upstream"] url = https://github.com/neovim/neovim.git fetch = +refs/heads/*:refs/remotes/upstream/* fetch = +refs/pull/*/head:refs/pull/upstream/* Then when you `git fetch --all`, you will have ALL pull requests available in your local repo in the local pull/ namespace. To check out PR #42: git checkout -b foo refs/pull/upstream/42

Mithaldu 12y ago
That is great, and i thank you for bringing that to my, and everyone else's attention! :)
jbish 12y ago
Yes! I've been wanting to do this and didn't know it was this easy.
ChuckMcM 12y ago
Wow you are my new favorite person. What I was going to say was that the GP was correct in that it just took a bit more work on the maintainers part to keep the history clean, but even that is unnecessary apparently :-)
hacknat 12y ago
You rock!
eiopa 12y ago
Good tip. It's interesting that other systems has a nicer implementation for it. In Phabricator, it's: $ arc patch D12345 and it checks out revision 12345 into an appropriately named branch.
zak_mc_kracken 12y ago
This is... amazing. Thank you so much.
12y ago

CHY872 12y ago

Just out of curiosity, have you ever used a tool such as Gerrit or Atlassian Stash? There are different mindsets, and both have their advantages.

Mithaldu 12y ago
I must admit i have not, and to make clear: I am not taking offense to their implication that Github is worse at some things than other tools (it is). My post above is merely objecting to the opinions that were borne out of some lack of knowledge (which is acceptable), yet are posited as facts (which is not).
- CHY872 12y ago
  Mm, I'm not sure. These are slides, presumably designed to go alongside a talk, which could have fleshed out the points better. If you consider them as being summaries of what a speaker's saying, it makes much more sense. Fwiw, of the six bullets, I see three which are clearly subjective, one which is a clear advantage of gerrit, and two which I don't really know enough about to refute. When it comes down to build systems, I think it's very easy to spend a lot of time moving sideways or backwards - and if the tool you're moving to has deficiencies compared with what you had before, it can be very frustrating. Certainly, if the tool people want you to move to offers few advantages over your current infrastructure, a quick dismissal is reasonable. Getting set up with their current contribution system might seem a little clunky, but I don't think it's particularly hard to do, and definitely seems like a 'run-once' thing. Once it's set up, it seems to integrate into a workflow well. Their contributors will have their current workflows set up nicely - and unless github's issues system has compelling advantages, it's definitely not worth them switching due to the temporary loss in productivity.

bdarnell 12y ago

>> Comments are sent as they are written; you cannot "draft" comments. > How is that different from pull requests via emails? (Also, on the website itself the comments can be edited.) With github PRs, notifications are sent as soon as you write your first comment; in systems like gerrit and rietveld (and email) they are not sent until the reviewer chooses to send them. This leads to either awkward interactions if you start replying to comments while the reviewer is still reviewing, or unresponsiveness if everyone introduces hysteresis to avoid this situation. This is IMHO the biggest problem with Github PRs; I don't necessarily agree with the Go team's decision to abandon the PR system but on smaller projects I have pushed for rietveld over PRs for this reason.

cbr 12y ago
My favorite are the comments like: 1:12pm line 33: Why are you doing this? 1:13pm line 33: I see, sorry, ignore my earlier comment. A system that lets you draft comments and then send them out in a batch can avoid a bunch of noise. On the other hand, though, people who aren't expecting it can get stuck with draft comments they don't send out.
- amenghra 12y ago
  I think we have the technology to show that a piece of text is a draft and hasn't yet been sent. Imho, the end user should be able to choose between live commenting vs draft+send.
- Gankro 12y ago
  Usually if this happens, it's a good hint that this piece of code could use a good comment. Not always, of course.

pjc50 12y ago

Don't use the merge button, just This is the smell of git plumbing again. Don't use the obvious UX that's been presented to you, do some other workflow that doesn't appear in the documentation.

jrochkind1 12y ago
Well, it depends on what documentation you mean. The alternative is using standard git commands that are in the git documentation, which is exactly what OP's alternative presumably involves? (But I am entirely sympathetic to the opinion that git itself exposes too much plumbing and has a pretty baroque end-user interface for doing certain things. And it's also certainly legit to wish or suggest that _github_'s UI worked differently than it does, although the merge commits don't really bother me, and some people prefer them, it's a point of some contention).
xkarga00 12y ago
But really... stop using the merge button.
rdtsc 12y ago
I see you've been downvoted and I think unfairly. I agree with you though. Looking back at arguments people make about how to "do it properly", it looks like something is broken. Git is broken, maybe Github is broken, documentation, UI, marketing. Something is though. When an obvious UI element is there, seemingly designed to do merges, and then everyone says "no, no, do this other thing", like send emails, then rebase here on top of that, make a ref pattern in your ~/.gitconfig ...
lomnakkus 12y ago
Git != github. (Does this really need to be said?)
- rdtsc 12y ago
  Saddly it does. There is probably a significant number of developers who never used git without Github. As a funny aside, when Github breaks the thing to say is "I wish one day someone would invent a decentralized version control system" ;-)

deleted 12y ago

[deleted]

enneff 12y ago

Half your post is saying we should just use different tools, but we are. We're using Gerrit, that lets us work the way we want. On the contributor instructions: note that if you just want to use plain git (and not our git-codereview tool) then you can stop at "Register with Gerrit." We created the tool to provide a more familiar review process for the people that used our previous system. If we took your advice and used offline diff tools, etc, we would need a similar page explaining how to do all of that. None of this convenience comes without an up front cost.

Mithaldu 12y ago
Thanks for the direct answer. I think my stance wasn't clear enough, so i'll reword what i wrote in another comment here. My issues are two-fold: You could have just said on that slide: "We want a central code review system, so people do not have to learn Git. Github isn't terrible, but Gerrit is much better." Instead you ended up putting up a list of points that make github seem like some kind of fatally flawed thing, while frankly putting people off with inaccuracies/subjectivities. Secondly, by not allowing things by github you're forcing people to learn something else. Most developers experienced with Git will also be very familiar with Github. You're telling those people to instead go and learn something else. That will result in some people deciding it's not worth the trouble. I'm fully aware it's up to you to decide whether you're willing to pay that price, but personally i find it a bit odd that you can't simply do both. And as for, apparently, most of the documentation on your contribute page being safely ignorable: If that is truly the case, i recommend rewriting that to make it obvious, because right now it's anything but. :) If the last bit is the only good thing that comes out of this, then i'll be happy.
- farresito 12y ago
  If they need to learn something new, they will. It's part of being a programmer. For me, github's way of doing things is way more flawed that gerrit.

zak_mc_kracken 12y ago

It's not exactly the first the time Go team is showing that they haven't spent much time researching current practices, theory, research and tools in the field of software engineering. Everything about the language screams of coming from minds who stopped learning new things in the late 90s.

dsymonds 12y ago
I think you mean s/researching/adopting/, since you are only referring to the outcome, not the study or decision making that arrived at the outcome. The Go team generally knows about modern things; we deliberately choose not to use them when we don't think they suit Go.
themightykevdog 12y ago
This is so colossally misguided a comment it's hard to know where to begin. So I won't.
- zak_mc_kracken 12y ago
  See my comment about "not wanting to learn". Happy to engage in a discussion about modern software engineering. Are you?

thockingoog 12y ago

I'll concur that the GitHub code-review UX is atrocious. Having used a small number of other code review tools, I can't fathom how GitHub's tooling is so brain-damaged. It does a few cute and clever things, but I totally agree with this slide deck's gripes. Email and GitHub are not really compatible.

hueving 12y ago

These are all common issues with people that came from a different CVS and didn't learn what was different about git. They just learn enough to reach parity with the "everything is linearly developed" workflow. When you live with that assumption, things like merge commits and pull requests seem silly and overkill since you live in the "patch is developed/reviewed in isolation" world.