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