opensource.google.com

Menu

Docs

Third-Party Reviewers

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, or PATENTS 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.

For CLs that affect Piper:

  1. Should I review this CL?
  2. Is this in the correct directory?
  3. Is the LICENSE file accurate?
  4. Is the METADATA file compliant?
  5. Does the BUILD file specify license type and export the LICENSE?
  6. Does the OWNERS file list two full-time Googlers?
  7. Are all the required files in the same directory?
  8. Does this CL need a language reviewer?
  9. Is there more than one version?

For reviews in Gerrit:

  1. Should I review this change?
  2. Is the LICENSE file accurate?
  3. 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.

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 a copyright statement and license.

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 by adding license-escalation to the reviewers (see escalations).

WARNING: Some licenses (such as AGPL, WTFPL, and more are never allowed at Google (unless it’s 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 the URL type is PIPER.
    • A last_upgrade_date field, if the url type isn’t PIPER.

(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.

(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.

Are all the required files in the same directory?

The LICENSE, METADATA, BUILD, and OWNERS files must all be in the same directory.

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, 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 vendor_src and google-vendor_src_branch documented 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.