Documentation: improve Diffusion documentation

.divinerconfig

@@ -4,8 +4,8 @@
   "groups" : {
     "intro" : "Introduction",
     "config" : "Configuration",
-    "contrib" : "Contributing",
     "userguide" : "Application User Guides",
+    "contrib" : "Contributing",
     "flavortext" : "Flavor Text",
     "developer" : "Phabricator Developer Guides",
     "differential" : "Differential (Code Review)",

src/__celerity_resource_map__.php

@@ -227,7 +227,7 @@ celerity_register_resource_map(array(
   ),
   'diffusion-source-css' =>
   array(
-    'uri' => '/res/494e1dd2/rsrc/css/application/diffusion/diffusion-source.css',
+    'uri' => '/res/415aad1a/rsrc/css/application/diffusion/diffusion-source.css',
     'type' => 'css',
     'requires' =>
     array(

src/applications/diffusion/controller/base/DiffusionController.php

@@ -42,6 +42,16 @@ abstract class DiffusionController extends PhabricatorController {
     $page->setBaseURI('/diffusion/');
     $page->setTitle(idx($data, 'title'));
     $page->setGlyph("\xE2\x89\x88");
+    $page->setTabs(
+      array(
+        'help' => array(
+          'href' => PhabricatorEnv::getDoclink(
+            'article/Diffusion_User_Guide.html'),
+          'name' => 'Help',
+        ),
+      ),
+      null);
+
     $page->appendChild($view);
 
     $response = new AphrontWebpageResponse();

src/applications/diffusion/controller/base/__init__.php

@@ -10,6 +10,7 @@ phutil_require_module('phabricator', 'aphront/response/webpage');
 phutil_require_module('phabricator', 'applications/base/controller/base');
 phutil_require_module('phabricator', 'applications/diffusion/request/base');
 phutil_require_module('phabricator', 'applications/diffusion/view/base');
+phutil_require_module('phabricator', 'infrastructure/env');
 phutil_require_module('phabricator', 'view/layout/crumbs');
 phutil_require_module('phabricator', 'view/layout/sidenav');
 

src/docs/general_coding_standards.diviner

@@ -1,18 +1,27 @@
 @title General Coding Standards
 @group contrib
 
-This document is a general coding standard for contributing to Phabricator, Arcanist, libphutil and Diviner.
-
+This document is a general coding standard for contributing to Phabricator,
+Arcanist, libphutil and Diviner.
 
 = Overview =
 
-This document contains practices and guidelines which apply across languages. Contributors should follow these guidelines. These guidelines are not hard-and-fast but should be followed unless there is a compelling reason to deviated from them.
-
+This document contains practices and guidelines which apply across languages.
+Contributors should follow these guidelines. These guidelines are not
+hard-and-fast but should be followed unless there is a compelling reason to
+deviated from them.
 
 = Code Complexity =
 
-  - Prefer to write simple code which is easy to understand. The simplest code is not necessarily the smallest, and some changes which make code larger (such as decomposing complex expressions and choosing more descriptive names) may also make it simpler. Be willing to make size tradeoffs in favor of simplicity.
-  - Prefer simple methods and functions which take a small number of parameters. Avoid methods and functions which are long and complex, or take an innumerable host of parameters. When possible, decompose monolithic, complex methods into several focused, simpler ones.
+  - Prefer to write simple code which is easy to understand. The simplest code
+    is not necessarily the smallest, and some changes which make code larger
+    (such as decomposing complex expressions and choosing more descriptive
+    names) may also make it simpler. Be willing to make size tradeoffs in favor
+    of simplicity.
+  - Prefer simple methods and functions which take a small number of parameters.
+    Avoid methods and functions which are long and complex, or take an
+    innumerable host of parameters. When possible, decompose monolithic, complex
+    methods into several focused, simpler ones.
   - Avoid putting many ideas on a single line of code.
 
 For example, avoid this kind of code:
@@ -42,11 +51,17 @@ And, obviously, don't do this sort of thing:
 = Performance =
 
   - Prefer to write efficient code.
-  - Strongly prefer to drive optimization decisions with hard data. Avoid optimizing based on intuition or rumor if you can not support it with concrete measurements.
-  - Prefer to optimize code which is slow and runs often. Optimizing code which is fast and runs rarely is usually a waste of time, and can even be harmful if it makes that code more difficult to understand or maintain. You can determine if code is fast or slow by measuring it.
+  - Strongly prefer to drive optimization decisions with hard data. Avoid
+    optimizing based on intuition or rumor if you can not support it with
+    concrete measurements.
+  - Prefer to optimize code which is slow and runs often. Optimizing code which
+    is fast and runs rarely is usually a waste of time, and can even be harmful
+    if it makes that code more difficult to understand or maintain. You can
+    determine if code is fast or slow by measuring it.
   - Reject performance discussions that aren't rooted in concrete data.
 
-In Phabricator, you can usually use the builtin XHProf profiling to quickly gather concrete performance data.
+In Phabricator, you can usually use the builtin XHProf profiling to quickly
+gather concrete performance data.
 
 
 = Naming Things =
@@ -83,9 +98,15 @@ Prefer these:
 = Error Handling =
 
   - Strongly prefer to detect errors.
-  - Strongly prefer to fail fast and loudly. The maximum cost of script termination is known, bounded, and fairly small. The maximum cost of continuing script execution when errors have occurred is unknown and unbounded. This also makes APIs much easier to use and problems far easier to debug.
+  - Strongly prefer to fail fast and loudly. The maximum cost of script
+    termination is known, bounded, and fairly small. The maximum cost of
+    continuing script execution when errors have occurred is unknown and
+    unbounded. This also makes APIs much easier to use and problems far easier
+    to debug.
 
-When you ignore errors, defer error handling, or degrade the severity of errors by treating them as warnings and then dismissing them, you risk dangerous behavior which may be difficult to troubleshoot:
+When you ignore errors, defer error handling, or degrade the severity of errors
+by treating them as warnings and then dismissing them, you risk dangerous
+behavior which may be difficult to troubleshoot:
 
   COUNTEREXAMPLE
   exec('echo '.$data.' > file.bak');                //  Bad!
@@ -105,7 +126,8 @@ Instead, fail loudly:
   }
   do_something_dangerous();
 
-But the best approach is to use or write an API which simplifies condition handling and makes it easier to get right than wrong:
+But the best approach is to use or write an API which simplifies condition
+handling and makes it easier to get right than wrong:
 
   execx('echo %s > file.bak', $data);               //  Good
   do_something_dangerous();
@@ -117,5 +139,8 @@ See @{article:System Commands} for details on the APIs used in this example.
 
 = Documentation, Comments and Formatting =
 
-  - Prefer to remove code by deleting it over removing it by commenting it out. It shall live forever in source control, and can be retrieved therefrom if it is ever again called upon.
-  - In source code, use only ASCII printable characters plus space and linefeed. Do not use UTF-8 or other multibyte encodings.
+  - Prefer to remove code by deleting it over removing it by commenting it out.
+    It shall live forever in source control, and can be retrieved therefrom if
+    it is ever again called upon.
+  - In source code, use only ASCII printable characters plus space and linefeed.
+    Do not use UTF-8 or other multibyte encodings.

src/docs/managing_daemons.diviner

@@ -63,8 +63,5 @@ You can get a list of launchable daemons with **phd list**:
   - **PhabricatorMetaMTADaemon** sends mail in the background, see
     @{article:Configuring Outbound Email} for details;
   - **PhabricatorTaskmasterDaemon** runs a generic task queue; and
-  - **PhabricatorRepository** daemons track repositories.
-
-TODO: Documentation on repository daemons is coming soon. The quick version is:
-run **phd repository-launch-master** after configuring them in the
-**Repositories** tool, then launch a few **taskmaster** daemons.
+  - **PhabricatorRepository** daemons track repositories, descriptions are
+    available in the @{article:Diffusion User Guide}.

src/docs/userguide/differential_test_plans.diviner

@@ -0,0 +1,66 @@
+@title Differential User Guide: Test Plans
+@group userguide
+
+This document describes things you should think about when developing a test
+plan.
+
+= Overview =
+
+When you send a revision for review in Differential you must include a test
+plan. A test plan is a repeatable list of steps which document what you have
+done to verify the behavior of a change. A good test plan convinces a reviewer
+that you have been thorough in making sure your change works as intended and
+has enough detail to allow someone unfamiliar with your change to verify its
+behavior.
+
+This document has some common things to think about when developing or reviewing
+a test plan. Some of these suggestions might not be applicable to the software
+you are writing; they are adapted from Facebook's internal documentation.
+
+= All Changes =
+
+  - **Error Handling:** Are errors detected and handled properly? How does your
+    change deal with error cases? Did you test them and make sure you got the
+    right error messages and the right behavior? It's important that you test
+    what happens when things go wrong, not just that the change works if
+    everything else also works.
+  - **Service Impact:** How does your change affect services like memcache,
+    thrift, or databases? Are you adding new cachekeys or queries? Will
+    this change add a lot of load to services?
+  - **Performance:** How does your change affect performance? **NOTE**: If
+    your change is a performance-motivated change, you should include profiles
+    in your test plan proving that you have improved performance.
+  - **Unit Tests:** Is your change adequately covered by unit tests? Could you
+    improve test coverage? If you're fixing a bug, did you add a test to prevent
+    it from happening again? Are the unit tests testing just the code in
+    question, or would a failure of a database or network service cause your
+    test to fail?
+  - **Concurrent Change Robustness:** If you're making a refactoring change, is
+    it robust against people introducing new calls between the time you started
+    the change and when you commit it? For example, if you change the parameter
+    order of some function from ##f(a, b)## to ##f(b, a)## and a new callsite is
+    introduced in the meantime, could it go unnoticed? How bad would that be?
+    (Because of this risk, you should almost never make parameter order
+    changes.)
+  - **Revert Plan:** If your change needs to be reverted and you aren't around,
+    are any special steps or considerations that the reverter needs to know
+    about? If there are, make sure they're adequately described in the "Revert
+    Plan" field so someone without any knowledge of your patch (but with a
+    general knowledge of the system) can successfully revert your change.
+  - **Security:** Is your change robust against XSS, CSRF, and injection
+    attacks? Are you verifying the user has the right capabilities or
+    permissions? Are you consistently treating user data as untrustworthy? Are
+    you escaping data properly, and using dangerous functions only
+    when they are strictly necessary?
+  - **Architecture:** Is this the right change? Could there be a better way to
+    solve the problem? Have you talked to (or added as reviewers) domain experts
+    if you aren't one yourself? What are the limitations of this solution? What
+    tradeoffs did you make, and why?
+
+= Frontend / User-Facing Changes =
+
+  - **Static Resources:** Will your change cause the application to serve more
+    JS or CSS? Can you use less JS/CSS, or reuse more?
+  - **Browsers:** Have you tested your change in multiple browsers?
+
+

src/docs/userguide/diffusion.diviner

@@ -0,0 +1,147 @@
+@title Diffusion User Guide
+@group userguide
+
+Guide to Diffusion, the Phabricator repository browser.
+
+= Overview =
+
+Diffusion is a repository browser which allows you to explore source code in a
+Git or SVN repository, similar to software like Trac and GitWeb.
+
+Diffusion provides a very high-performance SVN browser and a moderately
+high-performance Git browser. It achieves performance by denormalizing large
+amounts of data about repository history into a database and using this
+information like a cache so it can avoid querying the repository directly. This
+data is generated by daemons which track repositories, discover new commits, and
+parse and import them.
+
+Diffusion is integrated with the other tools in the Phabricator suite. For
+instance:
+
+  - when you commit Differential revisions to a tracked repository, they are
+    automatically updated and linked to the corresponding commits;
+  - you can add Herald rules to notify you about commits that match certain
+    rules;
+  - the Owners tool uses Diffusion to map repositories; and
+  - in all the tools, commit names are automatically linked.
+
+= Repository Callsigns and Commit Names =
+
+Each repository is identified by a "callsign", which is a short uppercase string
+like "P" (for Phabricator) or "ARC" (for Arcanist).
+
+Each repository must have a unique callsign. Callsigns must be unique within
+an install but do not need to be globally unique, so you are free to use the
+single-letter callsigns for brevity. For example, Facebook uses "E" for the
+Engineering repository, "O" for the Ops repository, "Y" for a Yum package
+repository, and so on, while Phabricator uses "P", "ARC", "PHU" for libphutil,
+and "J" for Javelin. Keeping callsigns brief will make them easier to use, and
+the use of one-character callsigns is recommended if they are reasonably
+evocative and you have no more than 26 tracked repositories.
+
+The primary goal of callsigns is to namespace commits to SVN repositories: if
+you use multiple SVN repositories, each repository has a revision 1, revision 2,
+etc., so referring to them by number alone is ambiguous. However, even for Git
+they impart additional information to human readers and allow parsers to detect
+that something is a commit name with high probability.
+
+Diffusion uses this callsign and information about the commit itself to generate
+a commit name, like "rE12345" or "rP28146171ce1278f2375e3646a1e1ea3fd56fc5a3".
+The "r" stands for "revision". It is followed by the repository callsign, and
+then a VCS-specific commit identifier (for SVN, the commit number; for Git, the
+commit hash). When writing the name of a Git commit you may abbreviate the hash,
+but note that hash collisions are probable for short prefix lengths. See this
+post on the LKML for a historical explanation of Git's occasional internal use
+of 7-character hashes:
+
+  https://lkml.org/lkml/2010/10/28/287
+
+Because 7-character hashes are likely to collide for even moderately large
+repositories, Diffusion generally uses either a 16-character prefix (which makes
+collisions very unlikely) or the full 40-character SHA1 (which makes collisions
+astronomically unlikely).
+
+= Adding Repositories =
+
+Repository administration is accomplished through the "Repository" tool, which
+is primarily a set of administrative interfaces for Diffusion. To add a
+repository to Diffusion, you need to:
+
+  - create a new repository in the Repository tool; and
+  - start the daemons that will track and import the repository.
+
+To create a new repository (or edit or delete an existing repository),
+**you must be an administrator** (see
+@{article:Configuring Accounts and Registration} for instructions on making an
+existing account an administrator account). As an administrator, go to the
+Repository tool and you'll have the options to create or edit repositories.
+
+When you create a new repository, you need to specify a human-readable name,
+a permanent "Callsign" (see previous section), and the underlying VCS type. Once
+you have created a repository, you can go to the "Tracking" tab and set up
+tracking in Diffusion.
+
+Most of the options in the **Tracking** tab should be self-explanatory or are
+safe to leave at their defaults. In broad strokes, Diffusion tracks SVN
+repositories by issuing an "svn log" command periodically against the remote to
+look for new commits. It tracks Git repositories by cloning a local copy and
+issuing "git fetch" periodically.
+
+Once you've configured everything (and made sure **Tracking** is set to
+"Enabled"), you can launch the daemons to begin actually tracking the
+repository.
+
+= Running Diffusion Daemons =
+
+For an introduction to Phabricator daemons, see
+@{article:Managing Daemons with phd}. To actually track repositories, you need
+to:
+
+  - run ##phd repository-launch-master## on one machine;
+  - run at least one @{class:PhabricatorTaskmasterDaemon} with
+    ##phd launch taskmaster##. You should probably launch a few of these
+    somewhere. They are generic workers which run many different kinds of
+    background tasks, so if you already have some running you don't need to
+    launch more. However, if you are importing a very large repository, import
+    rate will primarily be a function of how many taskmasters you are running so
+    you may want to launch a bunch of them; and
+  - if you have multiple web frontends and have tracked Git repositories, run
+    ##phd repository-launch-readonly## on each web frontend.
+
+You can use the Daemon Console to monitor the daemons and their progress
+importing the repository. Small repositories should import quickly, while
+larger repositories may take some time (it takes about 10 minutes to begin
+discovering commits in Facebook's 350,000-commit primary repository, and about
+18 hours to import it all with 64 taskmasters on modern hardware). Commits
+should begin appearing in Diffusion within a few minutes for all but the
+largest repositories.
+
+In detail, Diffusion uses several daemons to track, parse and import
+repositories:
+
+  - **PhabricatorRepositoryGitFetchDaemon**: periodically runs "git fetch" to
+    keep git repositories up to date
+  - **PhabricatorRepositoryGitCommitDiscoveryDaemon**: periodically looks for
+    new commits and imports them
+  - **PhabricatorRepositorySvnCommitDiscoveryDaemon**: periodically runs
+    "svn log" to look for new commits and import them
+  - **PhabricatorRepositoryCommitTaskDaemon**: creates tasks to parse and
+    import newly discovered commits
+
+The ##repository-launch-master## command just chooses the right daemons to
+launch based on which repositories you've configured to be tracked. If you add
+new repositories in the future, you should stop all the daemons and rerun
+##repository-launch-master##.
+
+If you run Phabricator with multiple web frontends, have your deployment script
+do a ##phd stop## and ##phd repository-launch-readonly## when it deploys. It is
+very unlikely you are impacted by this unless you are one of the largest
+installs in the world.
+
+= Building New Parsers =
+
+You can add new classes which will extend or enhance Diffusion's ability to
+parse commit messages.
+
+TODO: This is an advanced feature which doesn't currently have documentation and
+isn't terribly stable.

src/infrastructure/env/PhabricatorEnv.php

@@ -47,4 +47,8 @@ final class PhabricatorEnv {
     return self::$env;
   }
 
+  public static function getDoclink($resource) {
+    return 'http://phabricator.com/docs/phabricator/'.$resource;
+  }
+
 }