[Summary view] [Print] [Text view]

   1  @title Contributing Code
   2  @group detail
   3  
   4  Describes how to contribute code to Phabricator.
   5  
   6  Overview
   7  ========
   8  
   9  If you're planning to send a patch to Phabricator, this guide can help you
  10  through the process. The most important parts of contributing code to
  11  Phabricator are:
  12  
  13    - File a task with a bug report or feature request //before// you write code.
  14    - We rarely accept patches which we haven't discussed first.
  15    - We do not accept patches against prototype applications.
  16    - You must sign the CLA.
  17    - We do not accept GitHub pull requests.
  18    - Some alternative approaches are available if your change isn't something
  19      we want to bring upstream.
  20  
  21  The rest of this article describes these points in more detail, and then
  22  provides guidance on writing and submitting patches.
  23  
  24  If you just want to contribute some code but don't have a specific bug or
  25  feature in mind, see the bottom of this document for tips on finding ways to get
  26  started.
  27  
  28  For general information on contributing to Phabricator, see
  29  @{article:Contributor Introduction}.
  30  
  31  
  32  Coordinate First
  33  ================
  34  
  35  Before sending code, you should file a bug report or feature request describing
  36  what you'd like to write. For details on how to do this, see these articles:
  37  
  38    - @{article:Contributing Bug Reports}
  39    - @{article:Contributing Feature Requests}
  40  
  41  When you file a task, mention that you'd like to write the code to fix it. We
  42  can help contextualize your request or bug and guide you through writing an
  43  upstreamable patch, provided it's something that's upstreamable. If it isn't
  44  upstreamable, we can let you know what the issues are and help find another
  45  plan of attack.
  46  
  47  You don't have to file first (for example, if you spot a misspelling it's
  48  normally fine to just send a diff), but for anything even moderately complex
  49  you're strongly encouraged to file first and coordinate with the upstream.
  50  
  51  
  52  Rejecting Patches
  53  =================
  54  
  55  If you send us a patch without coordinating it with us first, it will probably
  56  be immediately rejected, or sit in limbo for a long time and eventually be
  57  rejected. The reasons we do this vary from patch to patch, but some of the most
  58  common reasons are:
  59  
  60  **Unjustifiable Costs**: We support code in the upstream forever. Support is
  61  enormously expensive and takes up a huge amount of our time. The cost to support
  62  a change over its lifetime is often 10x or 100x or 1000x greater than the cost
  63  to write the first version of it. Many uncoordinated patches we receive are
  64  "white elephants", which would cost much more to maintain than the value they
  65  provide.
  66  
  67  As an author, it may look like you're giving us free work and we're rejecting it
  68  as too expensive, but this viewpoint doesn't align with the reality of a large
  69  project which is actively supported by a small, experienced team. Writing code
  70  is cheap; maintaining it is expensive.
  71  
  72  By coordinating with us first, you can make sure the patch is something we
  73  consider valuable enough to put long-term support resources behind, and that
  74  you're building it in a way that we're comfortable taking over.
  75  
  76  **Not a Good Fit**: Many patches aren't good fits for the upstream: they
  77  implement features we simply don't want. You can find more information in
  78  @{article:Contributing Feature Requests}. Coordinating with us first helps
  79  make sure we're on the same page and interested in a feature.
  80  
  81  The most common type of patch along these lines is a patch which adds new
  82  configuration options. We consider additional configuration options to have
  83  an exceptionally high lifetime support cost and are very unlikely to accept
  84  them. Coordinate with us first.
  85  
  86  **Not a Priority**: If you send us a patch against something which isn't a
  87  priority, we probably won't have time to look at it. We don't give special
  88  treatment to low-priority issues just because there's code written: we'd still
  89  be spending time on something lower-priority when we could be spending it on
  90  something higher-priority instead.
  91  
  92  If you coordinate with us first, you can make sure your patch is in an area
  93  of the codebase that we can prioritize.
  94  
  95  **Overly Ambitious Patches**: Sometimes we'll get huge patches from new
  96  contributors. These can have a lot of fundamental problems and require a huge
  97  amount of our time to review and correct. If you're interested in contributing,
  98  you'll have more success if you start small and learn as you go.
  99  
 100  We can help you break a large change into smaller pieces and learn how the
 101  codebase works as you proceed through the implementation, but only if you
 102  coordinate with us first.
 103  
 104  **Generality**: We often receive several feature requests which ask for similar
 105  features, and can come up with a general approach which covers all of the use
 106  cases. If you send us a patch for //your use case only//, the approach may be
 107  too specific. When a cleaner and more general approach is available, we usually
 108  prefer to pursue it.
 109  
 110  By coordinating with us first, we can make you aware of similar use cases and
 111  opportunities to generalize an approach. These changes are often small, but can
 112  have a big impact on how useful a piece of code is.
 113  
 114  **Infrastructure and Sequencing**: Sometimes patches are written against a piece
 115  of infrastructure with major planned changes. We don't want to accept these
 116  because they'll make the infrastructure changes more difficult to implement.
 117  
 118  Coordinate with us first to make sure a change doesn't need to wait on other
 119  pieces of infrastructure. We can help you identify technical blockers and
 120  possibly guide you through resolving them if you're interested.
 121  
 122  
 123  No Prototype Changes
 124  ====================
 125  
 126  With rare exceptions, we do not accept patches for prototype applications for
 127  the same reasons that we don't accept feature requests or bug reports. To learn
 128  more about prototype applications, see
 129  @{article:User Guide: Prototype Applications}.
 130  
 131  
 132  You Must Sign the CLA
 133  =====================
 134  
 135  Before we can accept source code contributions, you need to submit a
 136  [[ https://secure.phabricator.com/L28 | Contributor License Agreement ]]. Your
 137  changes can not be accepted until you sign the agreement.
 138  
 139  If you haven't signed it by the time you send changes for review, you'll be
 140  reminded to sign it at that time.
 141  
 142  If you're submitting work on behalf of a company (like your employer), the
 143  company can sign the [[ https://secure.phabricator.com/L30 | Corporate
 144  Contributor License Agreement ]] instead.
 145  
 146  Both agreements are substantially similar to the Apache Foundation's CLAs. They
 147  protect Phacility and users of Phabricator by making sure we have permission to
 148  distribute your changes under an open source license.
 149  
 150  
 151  No Pull Requests
 152  ================
 153  
 154  We do not accept pull requests on GitHub:
 155  
 156    - We can not monitor who has signed CLAs on GitHub. You must sign the CLA
 157      to contribute, and we can't tell if you've signed it or not when you send
 158      us a pull request.
 159    - Pull requests do not get lint and unit tests run, so issues which are
 160      normally caught statically can slip by.
 161    - Phabricator is code review software, and developed using its own workflows.
 162      Pull requests bypass some of these workflows (for example, they will not
 163      trigger Herald rules to notify interested parties).
 164    - GitHub is not the authoritative master repository and we maintain a linear
 165      history, so merging pull requests is cumbersome on our end.
 166    - If you're comfortable enough with Phabricator to contribute to it, you
 167      should also be comfortable using it to submit changes.
 168  
 169  Instead of sending a pull request, use `arc diff` to create a revision on the
 170  upstream install. Your change will go through the normal Phabricator review
 171  process.
 172  
 173  (GitHub does not allow repositories to disable pull requests, which is why
 174  it's technically possible to submit them.)
 175  
 176  
 177  Alternatives
 178  ============
 179  
 180  If you've written code but we're not accepting it into the upstream, some
 181  alternative approaches include:
 182  
 183  **Maintain a local fork.** This will require some ongoing effort to port your
 184  changes forward when you update, but is often very reasonable for simple
 185  changes.
 186  
 187  **Develop as an application.** Many parts of Phabricator's infrastructure are
 188  modular, and modularity is increasing over time. A lot of changes can be built
 189  as external modules or applications without forking Phabricator itself. There
 190  isn't much documentation or support for this right now, but you can look at
 191  how other applications are implemented, and at other third-party code that
 192  extends Phabricator.
 193  
 194  **Rise to prominence.** We're more willing to accept borderline changes from
 195  community members who are active, make multiple contributions, or have a history
 196  with the project. This is not carte blanche, but distinguishing yourself can
 197  make us feel more comfortable about supporting a change which is slightly
 198  outside of our comfort zone.
 199  
 200  
 201  Writing and Submitting Patches
 202  ==================
 203  
 204  To actually submit a patch, run `arc diff` in `phabricator/`, `arcanist/`, or
 205  `libphutil/`. When executed in these directories, `arc` should automatically
 206  talk to the upstream install. You can add `epriestley` as a reviewer.
 207  
 208  You should read the relevant coding convention documents before you submit a
 209  change. If you're a new contributor, you don't need to worry about this too
 210  much. Just try to make your code look similar to the code around it, and we
 211  can help you through the details during review.
 212  
 213    - @{article:General Coding Standards} (for all languages)
 214    - @{article:PHP Coding Standards} (for PHP)
 215    - @{article:Javascript Coding Standards} (for Javascript)
 216  
 217  In general, if you're coordinating with us first, we can usually provide
 218  guidance on how to implement things. The other articles in this section also
 219  provide information on how to work in the Phabricator codebase.
 220  
 221  Not Sure Where To Get Started?
 222  ==============================
 223  
 224  If you don't have a specific bug or feature in mind and just want to write
 225  some code, you can try to find something simple to get started with.
 226  
 227  Because we're usually quick to fix easy bugs and issues, we often don't have a
 228  very good backlog of starter tasks.
 229  
 230  You can try searching in Maniphest for tasks tagged with #easy, which might
 231  have something, but a lot of time this list is small and the tasks on it aren't
 232  very fun or interesting even if they aren't technically too difficult.
 233  
 234  In general, the best way to contribute is to come to us with a problem you
 235  encountered or something you're interested in building, and then work with us
 236  to find a solution to it or a plan to build it. We can help turn a hacky patch
 237  into something that's upstreamable, and you'll get a fix or feature you want.
 238  
 239  You can also look though the rest of the open tasks for something more
 240  substantive that you're interested in. This will give you a better chance of
 241  finding something that's relevant to you, but many tasks are large or blocked
 242  by other large tasks.
 243  
 244  If you do find something, feel free to leave a comment like "I'm interested in
 245  working on this, is this something I could reasonably help with?". We're happy
 246  to walk through things, break larger tasks down into more detail, provide
 247  pointers to similar changes and the right places in the codebase to get started,
 248  and generally figure out how to attack a problem.
 249  
 250  
 251  Next Steps
 252  ==========
 253  
 254  Continue by:
 255  
 256    - returning to the @{article:Contributor Introduction}.