Skip to content

Latest commit

 

History

History
124 lines (94 loc) · 13.3 KB

contributing-code.md

File metadata and controls

124 lines (94 loc) · 13.3 KB

Contributing code and features

Thank you for helping make AMP better by fixing bugs, adding features or improving the AMP code in some other way!

This document describes the process you will go through to make a change in AMP.

Process Overview

Process for minor bug fixes and updates

We want to make it as easy as possible to get in small fixes. A fix for a small bug should be as easy as creating a PR with the change, adding/fixing a test, and sending it to a reviewer.

  • (optional) If you are fixing a bug and there is an existing GitHub issue, assign it to yourself or comment on it to let others know you are working on it. If there is no GitHub issue consider filing one, but for minor fixes your Pull Request description may be enough.
  • (optional) Find a reviewer before you start coding to help you answer questions. If you don't have any questions you can find a reviewer once you have a PR ready.
  • Follow the parts of the Implementation section that makes sense for your change. There are many parts of the process that you probably won't need to follow for a minor fix--e.g. you may not need to make validator changes or put your change behind an experiment for minor fixes. If in doubt ask your reviewer.

Process for significant changes

Significant changes (e.g. new components or significant changes to behavior) require consultation with and approval from knowledgeable members of the community.

  • Before you start coding, find a reviewer who you can discuss your change with and who can help guide you through the process.
  • File an Intent-to-implement (I2I) GitHub issue and cc your reviewer on it. The I2I should include:
    • A description of the change you plan to implement.
    • If you are integrating a third-party service, provide a link to the third-party's site and product.
    • Details on any data collection or tracking that your change might require.
    • A prototype or mockup (for example, an image, a GIF, or a link to a demo).
  • Determine who needs to approve your I2I. Changes that have a significant impact on AMP's behavior or significant new features require the approval from the Approvers Working Group (WG). Work with your reviewer to determine whether your change is significant enough that it requires approval from the Approvers Working Group and/or any other Working Group.
  • Get pre-approval from the Approvers WG if needed. For changes that require approval from the Approvers WG, at least 3 members of the Approvers WG should provide pre-approval on the I2I before significant implementation work proceeds.
  • Finalize the design of your change.
    • Familiarize yourself with our Design Principles.
    • Your reviewer can help you determine if your change requires a design doc and whether it should be brought to a design review.
  • Proceed with the implementation of your change.
  • For changes that require approval from the Approvers WG, file an Intent-to-ship (I2S) issue. Indicate which experiment is gating your change and a rollout plan. Once this issue is approved by 3 members of the Approvers WG the rollout plan described in the I2S may proceed.

Find a reviewer

Finding a reviewer for small changes

If you're making a minor fix and just want to find someone to review/approve your code, look at the OWNERS.yaml files in the directories where you're modifying code and either:

  • assign your PR to one of them if you are able to
  • cc them by adding the text "/cc @username" in the PR description/comment

Finding a reviewer for significant changes

A reviewer is needed if you are making a more substantial change to AMP. A reviewer is a member of the AMP community who is knowledgeable about the area you are modifying and who can guide you from the design phase all the way through launch.

To find a reviewer:

  • The Working Group that is most responsible for the area you are changing may document how to find a reviewer from that Working Group. If they do not, reach out to the facilitator of the WG (on Slack or by ccing them on your GitHub issue by adding "/cc @username" in the issue body or comment).
  • If there is no obvious Working Group responsible for the area you are changing but you know what part of the codebase your change will be in, reach out to one of the people in the OWNERS.yaml files for the areas you're changing (on Slack or by ccing them on your GitHub issue).
  • If you're still not sure who your reviewer should be, ask for a reviewer on Slack in the #contributing channel.
  • If you can't find a reviewer after going through these routes or the reviewers you find aren't responsive, reach out to mrjoro on Slack or cc him on your GitHub issue/PR.

Once you have found a reviewer, make sure to @-mention them on any issues / PRs related to your change (e.g. if mrjoro is your reviewer you can just add "/cc @mrjoro" in the issue/PR body or comment).

Implementation

Contributing extended components

AMP is designed to be extensible - it is meant to support “Extended Components” that provide first-class support for additional rich features.

Because Extended Components may have significant impact on AMP performance, security, and usage, Extended Component contributions will be very carefully analyzed and scrutinized.

In particular, we strive to design the overall component set, so that a large number of use cases can be composed from them. Instead of creating a new component it may thus be a better solution to combine existing components to a similar effect.

We have a few additional resources that provide an introduction to contributing extended components:

For further detail on integrating third-party services (e.g., fonts, embeds, etc.), see our 3p contribution guidelines.

Contributor License Agreement

AMP requires all contributors to either sign an individual Contributor License Agreement or be covered by a corporate Contributor License Agreement in order to protect contributors and users in issues of intellectual property.

We recommend you handle signing/being covered by a CLA before you send a PR to avoid problems, though this is not absolutely necessary until your code is ready to be merged in.

Make sure that the email you associate with your CLA is the same email address you associate with your commits (likely via the user.email Git config as described on GitHub's Set up Git page).

  • If you are contributing code on your own behalf you can sign the individual CLA instantly online.
  • If you are planning on contributing code on behalf of your company:
    • Your company will need to agree to a corporate CLA if it has not already done so. Although this is a relatively straightforward process, it requires approval from an authorized signer at your company and a manual verification process that may take a couple of days. To ensure you can get your code reviewed and merged quickly please start this process as soon as possible. The signer of your corporate CLA will associate a Google Group to the corporate CLA, and any email address added to this Google Group will be considered to be covered by this corporate CLA.
    • To be covered by your company's corporate CLA the owner of the Google Group associated with the corporate CLA (someone at your company) will need to add your address to this Google Group.
    • If you aren't sure whehther your company has already signed a corporate CLA, if you don't know who at your company owns the Google Group you need to be added to, or you run into any other questions about the corporate CLA, please reach out to @mrjoro (via cc on a PR or on Slack (signup)) or the #contributing channel on Slack.