This guide provides instructions for reviewers of third-party CLs including what to look for during reviews and how to escalate issues. If you’re just looking for instructions on submitting third-party code to google3, see go/thirdparty.
What’s involved and why would I want to help?
Reviewing additions to Google’s large cache of third-party code is rewarding and fun. It helps Google ensure we are staying in compliance with and being respectful of other authors’ licenses, and it also helps find security problems with outside code faster. It helps your fellow Googlers save time by using third-party code where it’s appropriate.
Depending on which reviewer groups you’re on, you may receive about a dozen or so CLs a week assigned to you for review. In the common case, they take only a few minutes to review. Some esoteric packages may require more review and/or escalation for more thorough legal review.
How reviewers are assigned
Third-party reviewers handle reviewing changes made in piper (most notably in
//third_party, but also a few other locations) as well as in various
Gerrit-on-Borg hosts (go/gob).
A presubmit defined in //piper/third_party/METADATA ensures that any CLs that touch one of a few files containing package metadata automatically adds emailremoved@ as a reviewer. A similar mechanism in Gerrit adds a special “Third-Party” label to any changes that touch third-party packages. An auto-assigner (go/gwsq) assigns a reviewer to these CLs, pulling from the membership of http://linkremoved/ in a round robin fashion.
CLs need third-party review in only a few situations—e.g., when a new package is
PATENTS files are modified. The
auto-assigner attempts to limit the number of CLs which are assigned reviewers
so as to avoid unnecessarily burdening them. In particular, a normal update to
//third_party needs no review. If you are assigned a CL which needs no review,
simply leave a comment but don’t give it approval (unless you’re the last
approver and the others state you need to approve it).
In some situations, you may want to assign a third-party reviewer even though
the auto-assigner claims one isn’t needed. To force the auto-assigner to assign
a one add
******************+force as a reviewer.
You may consider creating a bug Open Source > Compliance > Auto-Assigner to avoid this situation in the future, though we prefer to err on the side of over assigning rather than missing important reviews.
If you’d like to help with third-party reviews, email emailremoved@ and let them know. If you’ve already done that and have been instructed to move forward, then follow these steps to set things up.
Familiarize yourself with the relevant documents. In addition to this page, also read over:
Submit a proposal to be added to http://linkremoved/ (or other review group if applicable). Once added, you should start having CLs assigned to you within a day or so.
Your first few reviews
For your first few reviews, use Critique to add emailremoved@ to the R= line as soon as the review is assigned to you. He will help check your work. You can and should still go through the process as described below; just wait for emailremoved@’s LGTM before approving.
We review changes submitted through critique as well as Gerrit-on-Borg (go/gob). Most of the things we check for are the same, though there are a few procedural differences.
Many of these checks are performed by the compliance linter, which runs on every
//third_party CL, but we rely on the third-party reviewer to make a second
pass and ensure that any identified problems are corrected. You can normally
just click the “please fix” link on the suggestions that are left directly
inline in Critique like any other linter warning.
Is this review really for you?
Sometimes you will be assigned a review for a CL that doesn’t involve any new third-party code, but is rather a local modification to, or removal of, an existing package. In this case, what you do will depend on where the code is being committed:
- If the code is going into google3, you should not give an LGTM or approve the CL in this case. Doing so could allow the CL author to submit the code. You should allow the code’s real owners to do a review.
- If the code is going into a Gerrit host, you should give a +1 for Third-Party. Gerrit supports multiple approval dimensions, and unless the host is configured otherwise, a +1 on Third-Party is required but not sufficient to allow submission.
However, you should at least take a very brief look to make sure that
looks about how go/thirdparty says it should (of course, this doesn’t apply if
the entire package is being deleted). If it looks very strange, you may be
looking at a package so old that the rules for third-party code have changed
since its addition. In those cases, you should tell the submitter to look at
go/thirdparty and bring the package’s metadata up to date. Otherwise, just leave
a comment that says “no license compliance implications; custodial owners should
review and approve” or something like that, and let the package’s local owners
do the review.
Another time you might need to do the review yourself is if the package’s real
owners have all left the company. In this case, you will have to do a
traditional Google code review, and can pretty much disregard the rest of this
page. If this happens, you should insist that the submitter find two real
OWNERS for the package—one good suggestion is the submitter themselves,
Marking yourself inactive
The auto-assigner (go/gwsq) uses the same heuristic to determine if you’re on vacation as Critique. go/critique-vacation-detection#ooo-detection-heuristic
Occasionally, you may want to mark yourself as inactive for a period of time but
not be on vacation. To do that, add a
vacation() entry to
//piper/third_party/reviews.gwsq. See go/gwsq/vacations for more details.
There should be a file called
LICENSE containing a copyright statement and
license. This file must be named exactly
LICENCE, or anything like that (these alternatives may be present
in addition if they were part of the upstream distribution).
The vast majority of licenses are one of a few very common types (BSD, MIT, Apache, or GPL), and you will learn to recognize these licenses quickly. In other cases, you should always check go/thirdpartylicenses to see what type of license it is. If you don’t see the exact license listed there, do not attempt to figure it out yourself. Add emailremoved@ as a reviewer on the CL to have a lawyer review it.
WARNING: Code released under the GNU AGPL, which is different than the normal GPL, is not allowed to be used in Google products, ever. If you get a review for a CL adding a package that’s AGPL licensed (unless it is dual-licensed under less-restrictive terms, which is highly unlikely due to the AGPL’s nature and purpose), do not approve or LGTM it. Instruct the submitter to drop the CL and delete the code from their machine (see more at go/agpl).
Things to check for in the
namefield naming the code.
descriptionfield, giving a short description of the code.
third_partystructure field, with the appropriate sections in it.
urlfield. See go/thirdparty/metadata#url for examples of uses of this field.
versionfield, if the
last_upgrade_datefield, if the
BUILD file in Piper
NOTE: The following guidelines apply only to third-party code being checked into
Piper. Third party libraries in Gerrit repositories do not need a
BUILD must be present, even if the package doesn’t build with Blaze, and
must include, at a minimum,
licenses directive should specify the type of license listed
in go/thirdpartylicenses. For example, for the GPLv2:
# Foobar, a library for frobbing whoosies. licenses(['restricted']) exports_files(['LICENSE'])
OWNERS file in Piper
NOTE: The following guidelines apply only to third-party code being checked into
Piper; Third-party libraries in Gerrit repositories do not need an
OWNERS file must list at least two full-time Googlers explicitly as the
first two owners. That is, they need to be real usernames as opposed to
group: directives. When you view the CL in Critique, the usernames will be
links to http://linkremoved/ You should verify that the first two people are
full-timers, not interns (especially relevant during intern season). File and
group directives are permitted but the explicit people need to come first in the
The compliance linter will automatically detect if there are not two full-time Googlers listed as the first two owners in the OWNERS file. If the linter produces a warning and the author of the CL has not addressed it, please point them to go/thirdparty/where.
WARNING: Under no circumstances may an
OWNERS file under
include the line
See if the code needs a language reviewer
An http://linkremoved/ review checks for license compliance issues, but for
many projects there are additional guidelines to follow related to having code
in google3. There are special pools of volunteer reviewers for (most of) these,
so that each third-party reviewer doesn’t have to learn the rules for every
language or platform Google uses. When there is such a language review needed,
the rule is that both the auto-assigned
//third_party owner and the language
reviewer must look over the CL and give an LGTM. Whoever gives the second LGTM
should give the required g4 approval. This requires slightly more care on your
part but is necessary because there is no way to require multiple
approve something in g4.
- Languages that have their own reviewer group should be automatically added
to relevant CLs. If it looks like a group didn’t get added to a CL when it
should have been, you can manually add the reviewer group to the
R=line. Reviewers from these groups will be automatically assigned in the same way as ****************** reviewers, and will be identified in the CL comment left by change2review.
- If both a ****************** reviewer and a language reviewer has been assigned, the first reviewer should only LGTM, but not approve the CL. Typically, reviewers leave a comment saying “LGTM for compliance, ********************* will approve”, or something similar. The second reviewer should both LGTM and Approve once everything looks good.
- Some languages, such as C/C++ and Emacs Lisp, have some language-specific guidance, but not dedicated language reviewer group. See the language-specific pages in the sidebar of go/thirdparty to see what you should check for.
guidelines that are license compliance issues that need to be checked by
//third_party/OWNERSrelated to the
@licensetag being present in JSDoc
/**style comments that contain license or copyright notices.
Multiple versions of the same package
NOTE: Packages that have migrated to the new METADATA format will
not have a
MULTIPLE_VERSIONS_* file. Instead, this data is captured directly
Multiple simultaneous versions are highly discouraged (see go/oneversion). Any
exceptions to this rule always need to be documented with either a
MULTIPLE_VERSIONS_TEMPORARY (including a transition plan with timetables and
MULTIPLE_VERSIONS_OK (with justification accepted by the reviewers).
These files must also include a link to the emailremoved@ thread where this
multiple version exception was discussed and approved by the third-party team.
Except as otherwise noted, the content of this page is licensed under CC-BY-4.0 license. Third-party product names and logos may be the trademarks of their respective owners.