Add CONTRIBUTING.md
This CL creates a contributing document which lays out some information
to help contributors to the PDFium process.
Change-Id: I53ca2e9b3dbe28892070d7e3437fbbcb9bfaf04b
Reviewed-on: https://pdfium-review.googlesource.com/c/pdfium/+/70230
Commit-Queue: dsinclair <dsinclair@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Reviewed-by: Tom Sepez <tsepez@chromium.org>
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
new file mode 100644
index 0000000..4830470
--- /dev/null
+++ b/CONTRIBUTING.md
@@ -0,0 +1,195 @@
+# CONTRIBUTING
+In general, we follow the
+[Chromium Contributing](https://chromium.googlesource.com/chromium/src/+/master/docs/contributing.md)
+guidelines in PDFium. The code review process, and the build tools are all very
+similar to Chromium. The PDFium
+[README](https://pdfium.googlesource.com/pdfium/+/refs/heads/master/README.md)
+outlines specific build and test information for PDFium.
+
+This document focuses on how the PDFium project operates and how we’d like it
+to operate in the future. This is a living document, please file bugs if you
+think there are changes/updates which could be put in place to make it easier
+to contribute to PDFium.
+
+## Communication
+When writing a new feature or fixing an existing bug, get a second opinion
+before investing effort in coding. Coordinating up front makes it much easier
+to avoid frustration later on.
+
+If it‘s a new feature, or updating existing code, first propose it to the
+[mailing list](https://groups.google.com/forum/#!forum/pdfium).
+
+ * If a change needs further context outside the CL, it should be tracked in
+ the [bug system](https://bugs.chromium.org/p/pdfium). Bugs are the right
+ place for long histories, discussion and debate, attaching screenshots, and
+ linking to other associated bugs. Bugs are unnecessary for changes isolated
+ enough to not need any of these.
+ * If there isn't a bug and there should be one, please file a new bug.
+ * Just because there is a bug in the bug system doesn't necessarily mean that
+ a patch will be accepted.
+
+## Public APIs
+The public API of PDFium has grown over time. There are multiple mechanisms in
+place to support this growth from the stability requirements to the versioning
+fields. Along with those there are several other factors to be considered when
+adding public APIs.
+
+ * _Consistency_. We try to keep the APIs consistent with each other, this
+ includes things like naming, parameter ordering and how parameters are
+ handled.
+ * _Generality_. PDFium is used in several places outside the browser. This
+ could be server side, or in user applications. APIs should be designed to
+ work in the general case, or such that they can be expanded to the general
+ case if possible.
+ * _Documentation_. All public APIs should be documented to include information
+ on ownership of passed parameters, valid values being provided, error
+ conditions and return values.
+ * _Differentiate error conditions_. If at all possible, it should be possible
+ to tell the difference between a valid failure and an error.
+ * _Avoid global state_. APIs should receive the objects to be operated on
+ instead of assuming they exist in a global context.
+
+### Stability
+There are a lot of consumers of PDFium outside of Chromium. These include
+LibreOffice, Android and offline conversion tooling. As such, a lot of care is
+taken around the code in the
+[public](https://pdfium.googlesource.com/pdfium/+/refs/heads/master/public/)
+folder. When planning on changing the public API, the change should be preceded
+by a bug being created and an email to the mailing list to gather feedback from
+other PDFium embedders.
+
+The only stability guarantees that PDFium provides are around the APIs in the
+public folder. Any other interface in the system can be changed without notice.
+If there are features needed which are not exposed through the public headers
+you'll need to file a bug to get it added to the public APIs.
+
+#### Experimental
+All APIs start as Experimental. The experimental status is a documentation tag
+which is added to the API, the first line of the API documentation should be
+`// Experimental API.`
+
+Experimental APIs may be changed or removed entirely without formal notice to
+the community.
+
+#### Stable
+APIs eventually graduate to stable. This is done by removing the
+`// Experimental API.` marker in the documentation. We endeavor to not change
+stable APIs without notice to the community.
+
+NOTE, the process of migrating from experimental to stable isn’t well defined
+at this point. We have experimental APIs which have been that way for multiple
+years. We should work to better define how this transition happens.
+
+#### Deprecated
+If the API is retired, it is marked as deprecated and will eventually be removed.
+API deprecation should, ideally, come with a better replacement API and have a
+6-12 months deprecation period. The pending removal should be recorded in the
+documentation comment for the API and should also be recorded in the README with
+the target removal timeframe. All deprecations should have an associated bug
+attached to them.
+
+### Versioning
+In order to allow the public API to expand there are `version` fields in some
+structures. When the versioned structures are expanded those version fields
+need to be incremented to cover the new additions. The code then needs to guard
+against the structure being received having the required version number in
+order to validate the new additions are available.
+
+## Committers
+All changes committed to PDFium must be reviewed by a committer. Committers
+have done significant work in the PDFium code base and have a good overall
+understanding of the system.
+
+Contributors can become committers as they exhibit a strong understanding
+of the code base. There is a general requirement for ~10 non-trivial CLs to be
+written by the contributor before being considered for committership. The
+contributor is then nominated by an existing committer and if the nomination is
+accepted by two other committers they receive committer status.
+
+## OWNERS
+The OWNERS files list long time committers to the project and have a broad
+understanding of the code base and how the various pieces interact. In the
+event of a code review stalling with a committer, the OWNERS are the first line
+of escalation. The OWNERS files inherit up the tree, so an OWNER in a top-level
+folder has OWNERS in the folders subdirectories.
+
+There are a limited number of OWNERS files in PDFium at this time due to the
+inherent interconnectedness of the code. We are hoping to expand the number of
+OWNERS files to make them more targeted as the code quality progresses.
+
+Committers can be added to OWNERS files when they exhibit a strong
+understanding of the PDFium code base. This typically involves a combination of
+significant CLs, code review on other contributor CLs, and working with the
+other OWNERs to work through design and development considerations for the code.
+An OWNER must be committed to upholding the principles for the long term health
+of the project, take on a responsibility for reviewing future work, and
+mentor new contributors. Once you are a committer, you should feel free to reach
+out to the OWNERS who have reviewed your patches to ask what else they’d like to
+see from you to be comfortable nominating you as an OWNER. Once nominated,
+OWNERS are added or removed by rough consensus of the existing OWNERS.
+
+## Escalations
+There are times when reviews stall due to differences between reviewers,
+developers and OWNERS. If this happens, please escalate the situation to one of
+the people in the top-level OWNERS file (or another of the owners if already
+discussing with a top-level owner). If the disagreement has moved up through
+all the OWNERS files in the PDFium repo, the escalation should then proceed to
+the Chromium
+[ENG_REVIEW_OWNERS](https://chromium.googlesource.com/chromium/src/+/refs/heads/master/ENG_REVIEW_OWNERS)
+as the final deciders.
+
+The
+[Standard of Code Review](https://google.github.io/eng-practices/review/reviewer/standard.html)
+document has some good guidance on resolving conflicts during code review.
+
+## CLA
+All contributors must complete the Google contributor license agreement. For
+individual contributors, please complete the
+[Individual Contributor License Agreement](https://cla.developers.google.com/about/google-individual?csw=1)
+online. Corporate contributors must fill out the
+[Corporate Contributor License Agreement](https://cla.developers.google.com/about/google-corporate?csw=1)
+and send it to us as described on that page.
+
+Your first CL should add yourself to the
+[AUTHORS](https://pdfium.googlesource.com/pdfium/+/refs/heads/master/AUTHORS)
+file (unless you’re covered by one of the blanket entries).
+
+### External contributor checklist for reviewers
+Before LGTMing a change, ensure that the contribution can be accepted:
+ * Definition: The "author" is the email address that owns the code review
+ request on
+ [https://pdfium-review.googlesource.com](https://pdfium-review.googlesource.com/)
+ * Ensure the author is already listed in
+ [AUTHORS](https://pdfium.googlesource.com/pdfium/+/refs/heads/master/AUTHORS).
+ In some cases, the author's company might have a wildcard rule
+ (e.g. \*@google.com).
+ * If the author or their company is not listed, the CL should include a new
+ AUTHORS entry.
+ * Ensure the new entry is reviewed by a reviewer who works for Google.
+ * Contributor License Agreement can be verified by Googlers at
+ [http://go/cla](http://go/cla)
+ * If there is a corporate CLA for the author‘s company, it must list the
+ person explicitly (or the list of authorized contributors must say
+ something like "All employees"). If the author is not on their company’s
+ roster, do not accept the change.
+
+## Legacy Code
+The PDFium code base has been around in one form or another for a long time. As
+such, there is a lot of legacy hidden in the existing code. There are surprising
+interactions and untested corners of the code. We are actively working on
+increasing code coverage on the existing code, and especially welcome additions
+which move the coverage upwards. All new code should come with tests (either
+unit tests or integration tests depending on the feature).
+
+As part of this legacy nature, there is a good chance the code you’re working
+with wasn’t designed to do what you need it to do. There are often refactorings
+and bug fixes that end up happening along with feature development. Those
+fixes/refactorings should be pulled out to their own changes with the
+appropriate tests. This will make reviews a lot easier as, currently, it can be
+hard to tell if there are far reaching effects of a given change.
+
+There is a lot of existing technical debt that is being paid down in PDFium,
+anything we can do here to make future development easier is a great benefit to
+the project. This debt means means code reviews can take a bit longer if
+research is needed to determine how a feature change will interact with the
+rest of the system.
diff --git a/README.md b/README.md
index 76bbc60..cf603a5 100644
--- a/README.md
+++ b/README.md
@@ -240,14 +240,5 @@
## Contributing code
-For contributing code, we will follow
-[Chromium's process](https://chromium.googlesource.com/chromium/src/+/master/docs/contributing.md)
-as much as possible. The main exceptions are:
-
-1. Code has to conform to the existing style and not Chromium/Google style.
-2. PDFium uses a different Gerrit instance for code reviews, and credentials for
-this Gerrit instance need to be generated before uploading changes.
-
-Before submitting a fix for a bug, it can help if you create an issue in the
-bug tracker. This allows easier discussion about the problem and also helps
-with statistics tracking.
+See the [CONTRIBUTING](CONTRIBUTING.md) document for more information on
+contributing to the PDFium project.
