src/docs/contributor/contributing_code.diviner

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