Differential D25007 Diff 21 src/docs/contributor/contributing_code.diviner

Changeset View

Standalone View

src/docs/contributor/contributing_code.diviner

	@title Contributing Code			@title Contributing Code
	@group detail			@group detail

	Effective June 1, 2021: Phabricator is no longer actively maintained, and no longer accepting contributions.			Phorge is an open-source project, and welcomes contributions from the community
				at large. However, there are some guidelines we ask you to follow.


				Overview
				========

				The most important parts of contributing code to Phorge are:

				- File a task with a bug report or feature request //before// you write code.
				- We do not accept GitHub pull requests.
				- Some alternative approaches are available if your change isn't something
				we want to bring upstream.

				The rest of this article describes these points in more detail, and then
				provides guidance on writing and submitting patches.

				If you just want to contribute some code but don't have a specific bug or
				feature in mind, see the bottom of this document for tips on finding ways to get
				started.

				For general information on contributing to Phorge, see
				@{article:Contributor Introduction}.


				Coordinate First
				================

				Before sending code, you should file a task describing what you'd like to write.

				When you file a task, mention that you'd like to write the code to fix it. We
				can help contextualize your request or bug and guide you through writing an
				upstreamable patch, provided it's something that's upstreamable. If it isn't
				upstreamable, we can let you know what the issues are and help find another
				plan of attack.

				You don't have to file first (for example, if you spot a misspelling it's
				normally fine to just send a diff), but for anything even moderately complex
				you're strongly encouraged to file first and coordinate with the upstream.


				Rejecting Patches
				=================

				If you send us a patch without coordinating it with us first, it will probably
				be immediately rejected, or sit in limbo for a long time and eventually be
				rejected. The reasons we do this vary from patch to patch, but some of the most
				common reasons are:

				Unjustifiable Costs: We support code in the upstream forever. Support is
				enormously expensive and takes up a huge amount of our time. The cost to support
				a change over its lifetime is often 10x or 100x or 1000x greater than the cost
				to write the first version of it. Many uncoordinated patches we receive are
				"white elephants", which would cost much more to maintain than the value they
				provide.

				As an author, it may look like you're giving us free work and we're rejecting it
				as too expensive, but this viewpoint doesn't align with the reality of a large
				project which is actively supported by a small, experienced team. Writing code
				is cheap; maintaining it is expensive.

				By coordinating with us first, you can make sure the patch is something we
				consider valuable enough to put long-term support resources behind, and that
				you're building it in a way that we're comfortable taking over.

				Not a Good Fit: Many patches aren't good fits for the upstream: they
				implement features we simply don't want. Coordinating with us first helps
				make sure we're on the same page and interested in a feature.

				The most common type of patch along these lines is a patch which adds new
				configuration options. We consider additional configuration options to have
				an exceptionally high lifetime support cost and are very unlikely to accept
				them. Coordinate with us first.

				Not a Priority: If you send us a patch against something which isn't a
				priority, we probably won't have time to look at it. We don't give special
				treatment to low-priority issues just because there's code written: we'd still
				be spending time on something lower-priority when we could be spending it on
				something higher-priority instead.

				If you coordinate with us first, you can make sure your patch is in an area
				of the codebase that we can prioritize.

				Overly Ambitious Patches: Sometimes we'll get huge patches from new
				contributors. These can have a lot of fundamental problems and require a huge
				amount of our time to review and correct. If you're interested in contributing,
				you'll have more success if you start small and learn as you go.

				We can help you break a large change into smaller pieces and learn how the
				codebase works as you proceed through the implementation, but only if you
				coordinate with us first.

				Generality: We often receive several feature requests which ask for similar
				features, and can come up with a general approach which covers all of the use
				cases. If you send us a patch for //your use case only//, the approach may be
				too specific. When a cleaner and more general approach is available, we usually
				prefer to pursue it.

				By coordinating with us first, we can make you aware of similar use cases and
				opportunities to generalize an approach. These changes are often small, but can
				have a big impact on how useful a piece of code is.

				Infrastructure and Sequencing: Sometimes patches are written against a piece
				of infrastructure with major planned changes. We don't want to accept these
				because they'll make the infrastructure changes more difficult to implement.

				Coordinate with us first to make sure a change doesn't need to wait on other
				pieces of infrastructure. We can help you identify technical blockers and
				possibly guide you through resolving them if you're interested.


				No Prototype Changes
				====================

				With rare exceptions, we do not accept patches for prototype applications for
				the same reasons that we don't accept feature requests or bug reports. To learn
				more about prototype applications, see
				@{article:User Guide: Prototype Applications}.


				No Pull Requests
				================

				We do not accept pull requests on GitHub:

				- Pull requests do not get lint and unit tests run, so issues which are
				normally caught statically can slip by.
				- Phorge is code review software, and developed using its own workflows.
				Pull requests bypass some of these workflows (for example, they will not
				trigger Herald rules to notify interested parties).
				- GitHub is not the authoritative master repository and we maintain a linear
				history, so merging pull requests is cumbersome on our end.
				- If you're comfortable enough with Phorge to contribute to it, you
				should also be comfortable using it to submit changes.

				Instead of sending a pull request, use `arc diff` to create a revision on the
				upstream install. Your change will go through the normal Phorge review
				process.

				(GitHub does not allow repositories to disable pull requests, which is why
				it's technically possible to submit them.)


				Alternatives
				============

				If you've written code but we're not accepting it into the upstream, some
				alternative approaches include:

				Maintain a local fork. This will require some ongoing effort to port your
				changes forward when you update, but is often very reasonable for simple
				changes.

				Develop as an application. Many parts of Phorge's infrastructure are
				modular, and modularity is increasing over time. A lot of changes can be built
				as external modules or applications without forking Phorge itself. There
				isn't much documentation for this right now, but you can look at
				how other applications are implemented, and at other third-party code that
				extends Phorge.

				Rise to prominence. We're more willing to accept borderline changes from
				community members who are active, make multiple contributions, or have a history
				with the project. This is not carte blanche, but distinguishing yourself can
				make us feel more comfortable about supporting a change which is slightly
				outside of our comfort zone.


				Writing and Submitting Patches
				==================

				To actually submit a patch, run `arc diff` in `phorge/` or `arcanist/`.
				When executed in these directories, `arc` should automatically talk to the
				upstream install. You can add #blessed_reviewers as a reviewer.

				You should read the relevant coding convention documents before you submit a
				change. If you're a new contributor, you don't need to worry about this too
				much. Just try to make your code look similar to the code around it, and we
				can help you through the details during review.

				- @{article:General Coding Standards} (for all languages)
				- @{article:PHP Coding Standards} (for PHP)
				- @{article:Javascript Coding Standards} (for Javascript)

				In general, if you're coordinating with us first, we can usually provide
				guidance on how to implement things. The other articles in this section also
				provide information on how to work in the Phorge codebase.


				Next Steps
				==========

				Continue by:

				- returning to the @{article:Contributor Introduction}.