3 ms·

What is the better alternative Go is using? "Can only view diffs on a single page" You don't need to use GitHub to view diffs. "To create a patch one must fo

by nawitus 12y ago

What is the better alternative Go is using? "Can only view diffs on a single page" You don't need to use GitHub to view diffs. "To create a patch one must fork the repository publicly (weird and unnecessary)." I don't think it's weird. "Accepting a patch creates a "merge commit" (ugly repo history)." You don't need to have a merge commit, although I don't think that creates an ugly repo history. "In general, pull request culture is not about code review." I have no idea what this means.

admax88q 12y ago
They're probably using Gerrit. Although I agree, I don't think merge commits are ugly. I think people coming from SVN/CVS where history is strictly linear have this obsession with keeping it that way. In fact I find lots of developers just have an obsession with "clean" history, and fetishes for particular tools. It baffles me.
- notnowlewis 12y ago
  One of the previous slides says they are using Gerrit.
- sytse 12y ago
  GitLab CEO here, I completely agree with your observation that people coming from SVN have a hard time adjusting to non-linear history. At GitLab we recommend embracing it https://about.gitlab.com/2014/09/29/gitlab-flow/ https://about.gitlab.com/2014/09/29/gitlab-flow/ But we also believe in the freedom to do what you want, see there is a version of GitLab with automatic rebasing to allow a clean linear history without having to rebase by hand https://about.gitlab.com/2014/12/22/gitlab-7-6-and-ci-5-3-released/ https://about.gitlab.com/2014/12/22/gitlab-7-6-and-ci-5-3-re...
- Mithaldu 12y ago
  > I think people coming from SVN/CVS where history is strictly linear have this obsession with keeping it that way. This has absolutely nothing to do with those, especially since they didn't even allow cleaning up histories. There is one very simple reason for why some developers prefer a linear history of master: It makes debugging very easy. With branch merges, especially when the branch lines cross, or the merge is an octopus merge, the complexity of the code necessitating inspection to find the root cause of a bug straight-up explodes. Meanwhile with a linear history it's not only easy, but automatable to find a commit that breaks a thing. I do realize that you may not have had the displeasure yet to be in the situation to learn these things. Please feel free to consider yourself fortunate, but please also do try to understand that the things i just wrote are in fact simple observation of realities.
  - kasey_junk 12y ago
    git bisect is a great tool for automating this sort of debugging in non-linear histories. That said I agree in general linear histories are much easier to deal with.
  - lmm 12y ago
    Linearized histories are much harder to find bugs in. You end up looking through revisions that never existed; in SVN you merge the remote history into your local history without it ever showing up as a merge. The explicit git approach tells you something much closer to the truth. Your condescension is only hurting yourself.
    - Mithaldu 12y ago
      Your building strawmen does not help your credibility much, nor your attempt to convince me of your view. "Revisions that never existed" do exist, and if you put a rebase of a branch on master without verifying the rebase, then you may end up in a mess, but it's your fault. Code review is a thing that is done for a reason. Frankly, i find your style of argument through implication, and through trying to disregard something because it doesn't fit your definition of truth to be much more condescending than anything i wrote before.
      - MaulingMonkey 12y ago
        > "Revisions that never existed" do exist "Revision that were never built nor tested" is probably closer to the truth. Do you rewind through all your history and rebuild and retest every commit in a branch every time you rebase? Sure, they're similar, and you probably didn't mess up the merges. There's likely no subtle lingering bugs that QA's only going to catch weeks down the line. Probably. > then you may end up in a mess, but it's your fault. Code review is a thing that is done for a reason. Sure. But I've missed so many things in code reviews, had so many things in my own code missed in code reviews, and generally make mistakes and messes. I do agree that simpler branch topology tend to be easier to reason about. But branches do have their advantages... and I've also found that preserving the original branch topology has helped me untangle merge mistakes that were missed, committed, and then only discovered a year or more later.
MaulingMonkey 12y ago
> "In general, pull request culture is not about code review." > I have no idea what this means. Me either. To me, the entire point of pull requests is about triggering (code) reviews. If I don't want to bother with review, why not just grant direct push access and skip the rubber stamp ceremony?
- yawaramin 12y ago
  Pull request culture is about enabling distributed collaboration, i.e. I don't have push permission to your repo and you don't have push permission to mine, but we can still collaborate by sending each other pull requests and sharing our work that way. It's all about collaboration while not trusting the other person with write access to your repo.
  - MaulingMonkey 12y ago
    > It's all about collaboration while not trusting the other person with write access to your repo. If I don't trust someone enough to grant write access to my repo, I certainly don't trust them enough to rubber stamp their pull requests.
    - yawaramin 12y ago
      Nor should you. You treat their pull requests just as you should any code review: you study them and decide whether to approve (accept) them or not.
ptio 12y ago
> "In general, pull request culture is not about code review." Companies/organizations have different requirements for code reviews. Yes you can review code in pull requests, but in my opinion, it was not built for code reviews. In my previous company, we had a very rigorous code review process, which includes pre and post commit code reviews. We used Collaborator[1] and ReviewBoard[2] for code reviews and I totally understand why the Go team would use Gerrit over pull requests for code review. [1] http://smartbear.com/product/collaborator/overview/ http://smartbear.com/product/collaborator/overview/ [2] https://www.reviewboard.org/ https://www.reviewboard.org/

by nawitus 12y ago

admax88q 12y ago

They're probably using Gerrit. Although I agree, I don't think merge commits are ugly. I think people coming from SVN/CVS where history is strictly linear have this obsession with keeping it that way. In fact I find lots of developers just have an obsession with "clean" history, and fetishes for particular tools. It baffles me.

notnowlewis 12y ago
One of the previous slides says they are using Gerrit.
sytse 12y ago
GitLab CEO here, I completely agree with your observation that people coming from SVN have a hard time adjusting to non-linear history. At GitLab we recommend embracing it https://about.gitlab.com/2014/09/29/gitlab-flow/ https://about.gitlab.com/2014/09/29/gitlab-flow/ But we also believe in the freedom to do what you want, see there is a version of GitLab with automatic rebasing to allow a clean linear history without having to rebase by hand https://about.gitlab.com/2014/12/22/gitlab-7-6-and-ci-5-3-released/ https://about.gitlab.com/2014/12/22/gitlab-7-6-and-ci-5-3-re...
Mithaldu 12y ago
> I think people coming from SVN/CVS where history is strictly linear have this obsession with keeping it that way. This has absolutely nothing to do with those, especially since they didn't even allow cleaning up histories. There is one very simple reason for why some developers prefer a linear history of master: It makes debugging very easy. With branch merges, especially when the branch lines cross, or the merge is an octopus merge, the complexity of the code necessitating inspection to find the root cause of a bug straight-up explodes. Meanwhile with a linear history it's not only easy, but automatable to find a commit that breaks a thing. I do realize that you may not have had the displeasure yet to be in the situation to learn these things. Please feel free to consider yourself fortunate, but please also do try to understand that the things i just wrote are in fact simple observation of realities.
- kasey_junk 12y ago
  git bisect is a great tool for automating this sort of debugging in non-linear histories. That said I agree in general linear histories are much easier to deal with.
- lmm 12y ago
  Linearized histories are much harder to find bugs in. You end up looking through revisions that never existed; in SVN you merge the remote history into your local history without it ever showing up as a merge. The explicit git approach tells you something much closer to the truth. Your condescension is only hurting yourself.
  - Mithaldu 12y ago
    Your building strawmen does not help your credibility much, nor your attempt to convince me of your view. "Revisions that never existed" do exist, and if you put a rebase of a branch on master without verifying the rebase, then you may end up in a mess, but it's your fault. Code review is a thing that is done for a reason. Frankly, i find your style of argument through implication, and through trying to disregard something because it doesn't fit your definition of truth to be much more condescending than anything i wrote before.
    - MaulingMonkey 12y ago
      > "Revisions that never existed" do exist "Revision that were never built nor tested" is probably closer to the truth. Do you rewind through all your history and rebuild and retest every commit in a branch every time you rebase? Sure, they're similar, and you probably didn't mess up the merges. There's likely no subtle lingering bugs that QA's only going to catch weeks down the line. Probably. > then you may end up in a mess, but it's your fault. Code review is a thing that is done for a reason. Sure. But I've missed so many things in code reviews, had so many things in my own code missed in code reviews, and generally make mistakes and messes. I do agree that simpler branch topology tend to be easier to reason about. But branches do have their advantages... and I've also found that preserving the original branch topology has helped me untangle merge mistakes that were missed, committed, and then only discovered a year or more later.

MaulingMonkey 12y ago

> "In general, pull request culture is not about code review." > I have no idea what this means. Me either. To me, the entire point of pull requests is about triggering (code) reviews. If I don't want to bother with review, why not just grant direct push access and skip the rubber stamp ceremony?

yawaramin 12y ago
Pull request culture is about enabling distributed collaboration, i.e. I don't have push permission to your repo and you don't have push permission to mine, but we can still collaborate by sending each other pull requests and sharing our work that way. It's all about collaboration while not trusting the other person with write access to your repo.
- MaulingMonkey 12y ago
  > It's all about collaboration while not trusting the other person with write access to your repo. If I don't trust someone enough to grant write access to my repo, I certainly don't trust them enough to rubber stamp their pull requests.
  - yawaramin 12y ago
    Nor should you. You treat their pull requests just as you should any code review: you study them and decide whether to approve (accept) them or not.

ptio 12y ago

> "In general, pull request culture is not about code review." Companies/organizations have different requirements for code reviews. Yes you can review code in pull requests, but in my opinion, it was not built for code reviews. In my previous company, we had a very rigorous code review process, which includes pre and post commit code reviews. We used Collaborator[1] and ReviewBoard[2] for code reviews and I totally understand why the Go team would use Gerrit over pull requests for code review. [1] http://smartbear.com/product/collaborator/overview/ http://smartbear.com/product/collaborator/overview/ [2] https://www.reviewboard.org/ https://www.reviewboard.org/