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 http://linkremoved/
to
CLs with ********************
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
LICENSE
,OWNERS
,METADATA
, orPATENTS
files 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 LGTM
or Approval
from you.
Doing reviews
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.
General License Guidance
License review is one of the key components of the third party CL review. All code must have a non-prohibited license. Sometimes when reviewing a license, you may run into language, licenses, or an amount of licenses that go beyond the regular CL check process. When that happens, escalate to emailremoved@. For more information on licenses, please see What is a license?
License Review and Escalation
There are four main cases for escalation:
- Unknown license: If you see a license you do not recognize and cannot identify.
- Prohibited license: If you see a license that is on the prohibited list
- Non-standard license language: If the license appears to be a known license, but the language of the license does not match the known versions.
- Excessive license length: Some CLs import packages with a large number of licenses. When the number of licenses exceeds what you are personally comfortable reviewing, is over 1K lines, or is more than 10 licenses.
How to escalate
In the CL in question, add emailremoved@ to the CL reviewer line. License escalation will then review the CL in the order it comes up in their queue, generally within 1-2 business days
Once license escalation has completed their review and approved the CL, you may complete the rest of your review.
Review Protocol
For CLs that affect Piper:
- Should I review this CL?
- Get access to the code if needed
- 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?
- Is the directory structure correct?
- Does this CL need a language reviewer?
- Is there more than one version?
For reviews in Gerrit:
- Should I review this change?
- Get access to the code if needed
- Is the LICENSE file accurate?
- Is the METADATA file compliant?
NOTE: Some Gerrit hosts do not have the third-party reviewer flag configured. In these cases, just leave a comment stating approval.
How much time do I have?
Our documented target is first response within two business days, and a regular cadence of replies after that. But, we encourage reviewers to respond to CLs as fast as possible (i.e. within one day). New package additions are often blocking other work.
However, if the CL you are assigned also has a third-party-removed (shadowed) reviewer, please give the trainee a chance to perform a review first.
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
go/thirdparty/where#approved-third-party-removed. Within
//piper/.../third_party
, there are
language-specific subdirectories
where most new packages will be added.
The LICENSE file
There must be a file called LICENSE
containing an
allowed third party license. If the license text isn't something
that is recognizable, include emailremoved@ as a reviewer in the
CL. You can double check the license coverage by copy/pasting the LICENSE text
into http://linkremoved/
NOTE: This file must be named exactly LICENSE
, not LICENSE.txt, COPYING,
LICENCE, etc…
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.
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 METADATA
file:
- A
name
field naming the package. - A
description
field, giving a short description of the code. - A
third_party
structure field, with the appropriate sections in it.- A
url
field. See go/thirdparty/metadata#url for examples of uses of this field. - A
version
field, unless theURL
type isPIPER
. - A
last_upgrade_date
field, if theurl
type isn'tPIPER
. - Gerrit and google_vendor_src_branch: A
license_type
field.
- A
NB: Chromium third-party does not require a METADATA
file.
For versioned packages, the top-level METADATA
file should contain name
,
description
, and a sparse third-party struct that sets type: VERSIONS
, e.g.
name: "example-package"
description: "bindings to herd poodles over the internet"
third_party {
type: VERSIONS
}
(Piper Only) The BUILD file
There must be a BUILD
file (even if the package doesn't build with blaze) that
includes, at minimum:
exports_files('LICENSE')
licenses
directives
The licenses directive should specify the type of license listed in
go/thirdpartylicenses. For example, the BUILD
file for a packaged licensed
under GPLv2:
licenses(['restricted'])
exports_files(['LICENSE'])
The BUILD
file may also set the default_visibility
for the package. The
default for 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
package.
For versioned packages, the top-level BUILD
file generally exists to export
targets set by the BUILD
file in the versioned directory (though verification
of this is beyond the scope of third-party reviewers). The top-level BUILD
file does not need to export the LICENSE
file.
(Piper Only) The OWNERS file
The 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 file:
or
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 //third_party
include the line set noparent
.
Is the directory structure correct?
You'll need to ensure the LICENSE
, METADATA
, BUILD
and OWNERS
files are
in the correct location(s).
For an unversioned package all these files should be in a flat directory structure.
For a versioned package, the versioned directory will contain METADATA
,
BUILD
and LICENSE
files. The top-level package directory will contain
OWNERS
, and METADATA
files, and may contain a BUILD
file e.g.:
/third_party/example_package/BUILD
/third_party/example_package/METADATA
/third_party/example_package/OWNERS
/third_party/example_package/v1.0.0/BUILD
/third_party/example_package/v1.0.0/LICENSE
/third_party/example_package/v1.0.0/METADATA
The top-level BUILD
file, if present, may present an interface by which
dependent packages will depend upon this package. This ensures that dependent
packages do not require code changes on package updates in the general case.
This build file does not need to export the license file and should wrap the
appropriate build targets in the versioned directory's BUILD
file. This file
my be absent in the case of packages that have received multiple-version
exceptions. Particular languages may require a top-level BUILD
file.
The BUILD
file inside the versioned directory is the canonical third-party
BUILD
file and must conform to third-party standards.
METADATA
files in each of the top-level and versioned directories are
required, but each has different requirements.
If this is a new directory for multiple third-party packages, it must also
contain a README.txt
file. See go/thirdparty/where#subdirectories for more
information on the README.txt
file.
(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.
If gwsq
has assigned a third-party-removed reviewer and a language reviewer,
the first reviewer should only LGTM the CL (consider adding a WANT_LGTM
tag
to the CL). The second reviewer should both LGTM and Approve once everything
looks good.
NOTE: If the code is
JavaScript,
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, please reach out to 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
vendor_src
andgoogle-vendor_src_branch
documented at go/vendorsrc
You will be added as a shadowed reviewer to third-party CLs for a period of time before reviewing CLs on your own. During this period, perform a review as if you are a full third party owner to familiarize yourself to the whole process. Since you are the shadowed reviewer, please complete your review in a timely manner - the third party owner also assigned will be waiting for you to finish your initial round of the review before reviewing the CL themselves.
Once you have completed several reviews as a shadowed reviewer, please reach out to emailremoved@ to be evaluated for graduation to third-party-removed
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
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@.
How many reviews are being done?
Reviewers can see how many CLs are being assigned here: go/third-party-removed