3 ms·

You need to buy into the phab workflow which is diff based not branch based. Once you accept that the experience is awesome. For many people they don’t understa

by bogota 4y ago

You need to buy into the phab workflow which is diff based not branch based. Once you accept that the experience is awesome. For many people they don’t understand git enough unfortunately to appreciate the workflow not that it doesn’t have its flaws but it has worked amazingly on both large and small teams for me

2muchcoffeeman 4y ago
>You need to buy into the phab workflow which is diff based not branch based. What do you mean by this?
- capableweb 4y ago
  Quick overview of the differences: https://gist.github.com/StevenACoffman/cbb8e95960d2daac2077b0f163f99929 https://gist.github.com/StevenACoffman/cbb8e95960d2daac2077b...
- 10000truths 4y ago
  Phabricator works with patches, not branches. Kind of like how Linux kernel development works, except instead of attaching a patch file in an email and sending to a mailing list, the patch gets uploaded via a CLI tool (arc) and reviewed in a web interface.
  - deleted 4y ago
    [deleted]
- gorgoiler 4y ago
  When you do code review, you are approving a patch: the difference between the old code and the new code. With GitHub / Gitlab you push a branch and tell the tool to generate the diff by comparing two branches. With Phabricator you make the patch locally and push the patch directly for review. There is fooling to help you do this, as well as to pull and apply patches. This decouples Phabricator from your VCS tool and is partly a legacy of a time when Facebook had some engineers (and the central repo) using Subversion, and others using git. Even when everyone is using git, there are still 1001 different workflows which people use, so Phabricator ignores all that and zeroes in on the relevant bit: the code change you are proposing. It is more precise, imho. At its heart, code review is a human process for discussing the merits of a document that happens to be structured in the form of @@, + and - symbols. You can bolt other tools onto the side of it for CI/CD, linting, etc., but the human readable code change and social interaction parts are still front and centre.
  - boltzmann-brain 4y ago
    Ah, but you're not really approving a patch. You're approving a series of changes, and specifically their motivations. That's why git history is important. Complex code is meaningless without an understanding of what motivates it.
    - gorgoiler 4y ago
      Each patch has a form attached to it where you outline a title, description (including your motivation), and a test plan. These are mapped into the eventual commit message. Individual, atomic ideas are expressed as a single diff which maps to a single commit. Representing one idea as multiple commits is discouraged, though Phabricator does keep its own history of how a patch evolves should you ever need to refer to it.
    - jchw 4y ago
      The reviewable unit being as atomic as the version control unit has many positive knock-on effects. For example, if you merge numerous commits and keep the commit information, often times you will wind up with commits that have partial or complete breakage, which can seriously be a pain when someone is trying to triage a regression, for example. There is not one exact correct philosophy about how small the atomic unit is or what constitutes too much work, but the general wisdom is something to the order of "do one thing." I believe a PR should "do one thing," so PRs with multiple commits should be safe to flatten, else they likely contain too much. Of all of the things Google is rightly criticized for, their methodologies towards code review are pretty good and this is more or less what it felt like working in pseudo-Perforce. What if you needed multiple changes? Well, it was certainly more cumbersome than needed, but serieses work great here. Not only that, but patch serieses are already well established in the software engineering canon! Plenty of projects have accepted patch serieses over mailing lists for decades. I know many people have seen Github-style workflows work pretty well and so are unlikely to be receptive to the idea that some of the fundamentals could be wrong, but my advice is please consider it. It's not like there's one exactly correct way to handle merging changes, but I've found that the process of focusing on atomic sets of changes individually has improved my discipline as a software engineer. It may be a bit too wanky to call it "enlightening," but it's pretty close.

by bogota 4y ago

2muchcoffeeman 4y ago

>You need to buy into the phab workflow which is diff based not branch based. What do you mean by this?

capableweb 4y ago
Quick overview of the differences: https://gist.github.com/StevenACoffman/cbb8e95960d2daac2077b0f163f99929 https://gist.github.com/StevenACoffman/cbb8e95960d2daac2077b...
10000truths 4y ago
Phabricator works with patches, not branches. Kind of like how Linux kernel development works, except instead of attaching a patch file in an email and sending to a mailing list, the patch gets uploaded via a CLI tool (arc) and reviewed in a web interface.
- deleted 4y ago
  [deleted]
gorgoiler 4y ago
When you do code review, you are approving a patch: the difference between the old code and the new code. With GitHub / Gitlab you push a branch and tell the tool to generate the diff by comparing two branches. With Phabricator you make the patch locally and push the patch directly for review. There is fooling to help you do this, as well as to pull and apply patches. This decouples Phabricator from your VCS tool and is partly a legacy of a time when Facebook had some engineers (and the central repo) using Subversion, and others using git. Even when everyone is using git, there are still 1001 different workflows which people use, so Phabricator ignores all that and zeroes in on the relevant bit: the code change you are proposing. It is more precise, imho. At its heart, code review is a human process for discussing the merits of a document that happens to be structured in the form of @@, + and - symbols. You can bolt other tools onto the side of it for CI/CD, linting, etc., but the human readable code change and social interaction parts are still front and centre.
- boltzmann-brain 4y ago
  Ah, but you're not really approving a patch. You're approving a series of changes, and specifically their motivations. That's why git history is important. Complex code is meaningless without an understanding of what motivates it.
  - gorgoiler 4y ago
    Each patch has a form attached to it where you outline a title, description (including your motivation), and a test plan. These are mapped into the eventual commit message. Individual, atomic ideas are expressed as a single diff which maps to a single commit. Representing one idea as multiple commits is discouraged, though Phabricator does keep its own history of how a patch evolves should you ever need to refer to it.
  - jchw 4y ago
    The reviewable unit being as atomic as the version control unit has many positive knock-on effects. For example, if you merge numerous commits and keep the commit information, often times you will wind up with commits that have partial or complete breakage, which can seriously be a pain when someone is trying to triage a regression, for example. There is not one exact correct philosophy about how small the atomic unit is or what constitutes too much work, but the general wisdom is something to the order of "do one thing." I believe a PR should "do one thing," so PRs with multiple commits should be safe to flatten, else they likely contain too much. Of all of the things Google is rightly criticized for, their methodologies towards code review are pretty good and this is more or less what it felt like working in pseudo-Perforce. What if you needed multiple changes? Well, it was certainly more cumbersome than needed, but serieses work great here. Not only that, but patch serieses are already well established in the software engineering canon! Plenty of projects have accepted patch serieses over mailing lists for decades. I know many people have seen Github-style workflows work pretty well and so are unlikely to be receptive to the idea that some of the fundamentals could be wrong, but my advice is please consider it. It's not like there's one exactly correct way to handle merging changes, but I've found that the process of focusing on atomic sets of changes individually has improved my discipline as a software engineer. It may be a bit too wanky to call it "enlightening," but it's pretty close.