1# CONTRIBUTING 2In general, we follow the 3[Chromium Contributing](https://chromium.googlesource.com/chromium/src/+/main/docs/contributing.md) 4guidelines in PDFium. The code review process, and the build tools are all very 5similar to Chromium. The PDFium 6[README](https://pdfium.googlesource.com/pdfium/+/refs/heads/main/README.md) 7outlines specific build and test information for PDFium. 8 9This document focuses on how the PDFium project operates and how we’d like it 10to operate in the future. This is a living document, please file bugs if you 11think there are changes/updates which could be put in place to make it easier 12to contribute to PDFium. 13 14## Communication 15When writing a new feature or fixing an existing bug, get a second opinion 16before investing effort in coding. Coordinating up front makes it much easier 17to avoid frustration later on. 18 19If it‘s a new feature, or updating existing code, first propose it to the 20[mailing list](https://groups.google.com/forum/#!forum/pdfium). 21 22 * If a change needs further context outside the CL, it should be tracked in 23 the [bug system](https://bugs.chromium.org/p/pdfium). Bugs are the right 24 place for long histories, discussion and debate, attaching screenshots, and 25 linking to other associated bugs. Bugs are unnecessary for changes isolated 26 enough to not need any of these. 27 * If the work being implemented is especially complex or large a design 28 document may be warranted. The document should be linked to the filled bug 29 and be set to publicly viewable. 30 * If there isn't a bug and there should be one, please file a new bug. 31 * Just because there is a bug in the bug system doesn't necessarily mean that 32 a patch will be accepted. 33 34## Public APIs 35The public API of PDFium has grown over time. There are multiple mechanisms in 36place to support this growth from the stability requirements to the versioning 37fields. Along with those there are several other factors to be considered when 38adding public APIs. 39 40 * _Consistency_. We try to keep the APIs consistent with each other, this 41 includes things like naming, parameter ordering and how parameters are 42 handled. 43 * _Generality_. PDFium is used in several places outside the browser. This 44 could be server side, or in user applications. APIs should be designed to 45 work in the general case, or such that they can be expanded to the general 46 case if possible. 47 * _Documentation_. All public APIs should be documented to include information 48 on ownership of passed parameters, valid values being provided, error 49 conditions and return values. 50 * _Differentiate error conditions_. If at all possible, it should be possible 51 to tell the difference between a valid failure and an error. 52 * _Avoid global state_. APIs should receive the objects to be operated on 53 instead of assuming they exist in a global context. 54 55### Stability 56There are a lot of consumers of PDFium outside of Chromium. These include 57LibreOffice, Android and offline conversion tooling. As such, a lot of care is 58taken around the code in the 59[public](https://pdfium.googlesource.com/pdfium/+/refs/heads/main/public/) 60folder. When planning on changing the public API, the change should be preceded 61by a bug being created and an email to the mailing list to gather feedback from 62other PDFium embedders. 63 64The only stability guarantees that PDFium provides are around the APIs in the 65public folder. Any other interface in the system can be changed without notice. 66If there are features needed which are not exposed through the public headers 67you'll need to file a bug to get it added to the public APIs. 68 69#### Experimental 70All APIs start as Experimental. The experimental status is a documentation tag 71which is added to the API, the first line of the API documentation should be 72`// Experimental API.` 73 74Experimental APIs may be changed or removed entirely without formal notice to 75the community. 76 77#### Stable 78APIs eventually graduate to stable. This is done by removing the 79`// Experimental API.` marker in the documentation. We endeavor to not change 80stable APIs without notice to the community. 81 82NOTE, the process of migrating from experimental to stable isn’t well defined 83at this point. We have experimental APIs which have been that way for multiple 84years. We should work to better define how this transition happens. 85 86#### Deprecated 87If the API is retired, it is marked as deprecated and will eventually be removed. 88API deprecation should, ideally, come with a better replacement API and have a 896-12 months deprecation period. The pending removal should be recorded in the 90documentation comment for the API and should also be recorded in the README with 91the target removal timeframe. All deprecations should have an associated bug 92attached to them. 93 94### Versioning 95In order to allow the public API to expand there are `version` fields in some 96structures. When the versioned structures are expanded those version fields 97need to be incremented to cover the new additions. The code then needs to guard 98against the structure being received having the required version number in 99order to validate the new additions are available. 100 101## Trybot Access 102Changes must pass the try bots before they are merged into the repo. For your 103first few CLs the try bots will need to be triggered by a committer. After 104you've submitted 2-3 CLs you can request try bot access by emailing one of the 105OWNERS and requesting try bot access. This will allow you to trigger the bots 106on your own changes without needing a committer. 107 108## Committers 109All changes committed to PDFium must be reviewed by a committer. Committers 110have done significant work in the PDFium code base and have a good overall 111understanding of the system. 112 113Contributors can become committers as they exhibit a strong understanding 114of the code base. There is a general requirement for ~10 non-trivial CLs to be 115written by the contributor before being considered for committership. The 116contributor is then nominated by an existing committer and if the nomination is 117accepted by two other committers they receive committer status. 118 119## OWNERS 120The OWNERS files list long time committers to the project and have a broad 121understanding of the code base and how the various pieces interact. In the 122event of a code review stalling with a committer, the OWNERS are the first line 123of escalation. The OWNERS files inherit up the tree, so an OWNER in a top-level 124folder has OWNERS in the folders subdirectories. 125 126There are a limited number of OWNERS files in PDFium at this time due to the 127inherent interconnectedness of the code. We are hoping to expand the number of 128OWNERS files to make them more targeted as the code quality progresses. 129 130Committers can be added to OWNERS files when they exhibit a strong 131understanding of the PDFium code base. This typically involves a combination of 132significant CLs, code review on other contributor CLs, and working with the 133other OWNERs to work through design and development considerations for the code. 134An OWNER must be committed to upholding the principles for the long term health 135of the project, take on a responsibility for reviewing future work, and 136mentor new contributors. Once you are a committer, you should feel free to reach 137out to the OWNERS who have reviewed your patches to ask what else they’d like to 138see from you to be comfortable nominating you as an OWNER. Once nominated, 139OWNERS are added or removed by rough consensus of the existing OWNERS. 140 141## Escalations 142There are times when reviews stall due to differences between reviewers, 143developers and OWNERS. If this happens, please escalate the situation to one of 144the people in the top-level OWNERS file (or another of the owners if already 145discussing with a top-level owner). If the disagreement has moved up through 146all the OWNERS files in the PDFium repo, the escalation should then proceed to 147the Chromium 148[ATL_OWNERS](https://chromium.googlesource.com/chromium/src/+/refs/heads/main/ATL_OWNERS) 149as the final deciders. 150 151The 152[Standard of Code Review](https://google.github.io/eng-practices/review/reviewer/standard.html) 153document has some good guidance on resolving conflicts during code review. 154 155## CLA 156All contributors must complete the Google contributor license agreement. For 157individual contributors, please complete the 158[Individual Contributor License Agreement](https://cla.developers.google.com/about/google-individual?csw=1) 159online. Corporate contributors must fill out the 160[Corporate Contributor License Agreement](https://cla.developers.google.com/about/google-corporate?csw=1) 161and send it to us as described on that page. 162 163Your first CL should add yourself to the 164[AUTHORS](https://pdfium.googlesource.com/pdfium/+/refs/heads/main/AUTHORS) 165file (unless you’re covered by one of the blanket entries). 166 167### External contributor checklist for reviewers 168Before LGTMing a change, ensure that the contribution can be accepted: 169 * Definition: The "author" is the email address that owns the code review 170 request on 171 [https://pdfium-review.googlesource.com](https://pdfium-review.googlesource.com/) 172 * Ensure the author is already listed in 173 [AUTHORS](https://pdfium.googlesource.com/pdfium/+/refs/heads/main/AUTHORS). 174 In some cases, the author's company might have a wildcard rule 175 (e.g. \*@google.com). 176 * If the author or their company is not listed, the CL should include a new 177 AUTHORS entry. 178 * Ensure the new entry is reviewed by a reviewer who works for Google. 179 * Contributor License Agreement can be verified by Googlers at 180 [http://go/cla](http://go/cla) 181 * If there is a corporate CLA for the author‘s company, it must list the 182 person explicitly (or the list of authorized contributors must say 183 something like "All employees"). If the author is not on their company’s 184 roster, do not accept the change. 185 186## Legacy Code 187The PDFium code base has been around in one form or another for a long time. As 188such, there is a lot of legacy hidden in the existing code. There are surprising 189interactions and untested corners of the code. We are actively working on 190increasing code coverage on the existing code, and especially welcome additions 191which move the coverage upwards. All new code should come with tests (either 192unit tests or integration tests depending on the feature). 193 194As part of this legacy nature, there is a good chance the code you’re working 195with wasn’t designed to do what you need it to do. There are often refactorings 196and bug fixes that end up happening along with feature development. Those 197fixes/refactorings should be pulled out to their own changes with the 198appropriate tests. This will make reviews a lot easier as, currently, it can be 199hard to tell if there are far reaching effects of a given change. 200 201There is a lot of existing technical debt that is being paid down in PDFium, 202anything we can do here to make future development easier is a great benefit to 203the project. This debt means means code reviews can take a bit longer if 204research is needed to determine how a feature change will interact with the 205rest of the system. 206