This guide provides instructions on how to review third-party CLs. It includes what to look for during reviews, and how and when to escalate issues.
TIP: If you’re looking for instructions on submitting third-party code to google3, see go/thirdparty
Why review third-party code?
Reviewing additions to Google’s large cache of third-party code helps Google ensure we are using third-party code properly and respectfully.
Depending on your reviewer group, you may be assigned about a dozen CLs a week to review. Most of them should take only a few minutes to review. More complicated packages may require more review and/or escalation.
How reviewers are assigned
An auto-assigner (go/gwsq) assigns a reviewer from
******************** or with the “Third-Party” Gerrit label. These
flags are set by
METADATA files or Gerrit magic.
Does it need review?
Reviews are necessary in only a couple of situations:
- When a new package is added
- When the
PATENTSfiles are modified
Normal updates to existing packages often do not require re-review, but the
auto-assigner can’t deal with all the cases. If you are assigned a CL with no
compliance implications, just make a comment saying “no compliance
implications”. It should not require
Approval from you.
While some checks are performed by the compliance linter, we rely on the third-party reviewer to ensure the required metadata is present and accurate and ensure that any identified problems are corrected.
If you notice any issues that could have been detected automatically but weren’t, please file a go/compliance-linter-bug.
NOTE: The compliance linter doesn’t run on large CLs (>500 files) http://linkremoved/
At a high level, you should follow these checklists, depending on where the change is being done. You should also apply best practices from your knowledge of our documentation, Google standards, and common sense. If you see something that looks off, please bring it up in the CL comments.
For CLs that affect Piper:
- Should I review this CL?
- Is this in the correct directory?
- Is the LICENSE file accurate?
- Is the METADATA file compliant?
- Does the BUILD file specify license type and export the LICENSE?
- Does the OWNERS file list two full-time Googlers?
- Are all the required files in the same directory?
- Does this CL need a language reviewer?
- Is there more than one version?
For reviews in Gerrit:
NOTE: Some Gerrit hosts do not have the third-party reviewer flag configured. In these cases, just leave a comment stating approval.
Should I review this CL or change?
If this change is adding a new third-party package or changing its license, you should review this change.
If it is removing a third-party package, see go/thirdparty/reviewers#deleting for instructions.
Is this package in the correct directory?
There are multiple
third-party locations. You can see a full list at
//piper/.../third_party, there are
where most new packages will be added.
The LICENSE file
There must be a file called
LICENSE containing a copyright statement and
NOTE: This file must be named exactly
LICENSE, not LICENSE.txt, COPYING,
If the code is licensed under multiple licenses, see go/thirdparty/licenses#multiple for guidance.
If you do not recognize the license, check go/thirdpartylicenses to see what
type of license it is. If you don’t see the license listed at
go/thirdpartylicenses, escalate the CL by adding
license-escalation to the
reviewers (see escalations).
WARNING: Some licenses (such as AGPL and WTFPL; see go/thirdpartylicenses#banned) are never allowed at Google (unless the code is dual licensed under an allowed license). If you get a review for such a package, instruct the submitter to drop the CL and delete the code from their machine (see more at go/agpl).
The METADATA file
Things to check for in the
namefield naming the package.
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, unless the
last_upgrade_datefield, if the
(Piper Only) The BUILD file
There must be a
BUILD file (even if the package doesn’t build with blaze) that
includes, at minimum:
The licenses directive should specify the type of license listed in
go/thirdpartylicenses. For example, the
BUILD file for a packaged licensed
BUILD file may also set the
default_visibility for the package. The
default_visibility is private: go/thirdpartylicenses#visibility
NOTE: Visibility must be restricted when the package is licensed under a
by_exception_only license (see go/byexceptiononly) or when it is a versioned
(Piper Only) The OWNERS file
OWNERS file must list at least the usernames of two full-time Googlers
explicitly as the first two lines. The first two lines cannot be
group: directives, unless it is a
file: directive pointing to another
OWNERS file in
//third_party. Verify that the first two people are
employees, not interns or TVCs (especially relevant during intern season). File
and group directives are permitted but usernames must be first in the file.
WARNING: Under no circumstances may an
OWNERS file under
include the line
Are all the required files in the same directory?
OWNERS files must all be in the same
If this is a new directory for multiple third-party packages, it must also
README.txt file. See go/thirdparty/where#subdirectories for more
information on the
(Piper Only) Does this CL need a language reviewer?
Languages that have their own reviewer group should be automatically added to relevant CLs. If a group didn’t get added to a CL when it should have been, manually add the reviewer group to the CL. Some languages, such as C/C++ and Emacs Lisp, have some language-specific guidance, but no reviewer group.
See go/thirdparty/working#LanguageSpecificGuidance for reviewing their CLs.
gwsq has assigned a third-party-removed reviewer and a language reviewer,
the first reviewer should only LGTM the CL (consider adding a
to the CL). The second reviewer should both LGTM and Approve once everything
NOTE: If the code is
the third-party reviewer needs to check that the
@license tag is present in
JSDoc /** style comments that contain license.
(Piper Only) Is there more than one version?
Multiple simultaneous versions are highly discouraged (see go/oneversion).
All exceptions to this rule need to be documented with a
third_party.multiple_versions field entry in the
METADATA file. The author
must include a link to the third-party-removed thread where the multiple version
exception was discussed and approved by the third-party team. (A temporary
exception additionally requires an expiration entry.)
Deleting third-party packages
Ensure that the package owners are signing off on the deletion and leave a comment saying “no license compliance indications; custodial owners should review and approve” to indicate you reviewed the change.
NOTE: If there are no package owners left, you may approve the CL.
Getting started as a reviewer
If you’d like to help with third-party reviews, email emailremoved@ and ask to be onboarded.
Familiarize yourself with the contents of this page, as well as:
- Guidance for third-party submitters documented at go/thirdparty
- Open source licenses and their categories documented at go/thirdpartylicenses
- Notes on
google-vendor_src_branchdocumented at go/vendorsrc
You will be added as a shadow to third-party CLs for a period of time before reviewing CLs on your own.
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
If you want to explicitly mark yourself as inactive for a period of time
(without actual vacation), add a
vacation() entry to
//piper/third_party/reviews.gwsq. See go/gwsq-vacations for more details.
Need help? Just ask.
If you aren’t sure about something, make a comment in the right place and ask a more experienced third-party reviewer to take a look, or post a message to emailremoved@.
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.