Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

📖 Assorted issue and PR template fixes #34473

Merged
merged 5 commits into from
May 21, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions .github/ISSUE_TEMPLATE/release-tracker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,10 @@ body:
This is AMP's release tracker form, meant to be used by on-duty engineers.
- See AMP's [release schedule](https://github.com/ampproject/amphtml/blob/main/docs/release-schedule.md) to learn about how releases work.
- See AMP's [release calendar](https://amp-release-calendar.appspot.com) for information about past releases (e.g. versions, dates, submission times, notes).
- Use this form to track a release through the following stages:
- Initial promotion to Nightly channel
- Promotion to Experimental and Beta opt-in channels
- Promotion to Experimental and Beta 1% traffic channels
- Promotion to Stable channel
- Promotion to LTS channel (if applicable)
- See AMP's pre-release [documentation](https://github.com/ampproject/amphtml/blob/main/docs/release-schedule.md#beta-and-experimental-channels) to learn how to test changes in the Experimental channel.
- See AMP's [pre-release documentation](https://github.com/ampproject/amphtml/blob/main/docs/release-schedule.md#beta-and-experimental-channels) to learn how to test changes in the Experimental channel.
- If you find a bug in this release, file a [bug report](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=Type%3A+Bug&template=bug-report.yml).
- If you believe a bug should be fixed as part of this release, request a [cherry-pick](https://github.com/ampproject/amphtml/blob/main/docs/contributing-code.md#Cherry-picks).
- For updates that may be of interest to the community (e.g. bug fixes, delayed releases), post a comment to this issue.
- The community uses this issue to keep track of releases, so please keep it up to date.
- type: input
id: release_version
attributes:
Expand Down
39 changes: 0 additions & 39 deletions .github/PULL_REQUEST_TEMPLATE.md

This file was deleted.

34 changes: 34 additions & 0 deletions .github/PULL_REQUEST_TEMPLATE/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
<!--
# Instructions:

1. Pick a meaningful title for your pull request.
a. Prefix the title with an emoji. (Copy-paste from the list below.)
b. If helpful, use a short prefix (e.g. `[Project XX] Implement feature YY`).
2. Enter a description that explains why the PR is necessary, and what it does.
a. Mention the GitHub issue being addressed by this pull request.
b. Use keywords to auto-close linked issues during merge. (e.g. `Fixes #11111`, or `Closes #22222`)
3. For substantial changes, first file an Intent-to-Implement (I2I) issue at go.amp.dev/i2i.

# References:

- AMP code contribution docs: go.amp.dev/contribute/code
- First time setup (required for CI checks): go.amp.dev/getting-started

# Emojis for categorizing pull requests (copy-paste emoji into description):

✨ New feature
🐛 Bug fix
🔥 P0 fix
✅ Tests
❄️ Flaky tests
🚀 Performance improvements
🖍 CSS / Styling
♿ Accessibility
🌐 Internationalization
📖 Documentation
🏗 Infrastructure / Tooling / Builds / CI
⏪ Revert
♻️ Refactor
🚮 Deletion
🧪 Experimental code
-->
10 changes: 6 additions & 4 deletions build-system/pr-check/build-targets.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,10 @@ let buildTargets;
/**
* Used to prevent the repeated expansion of globs during PR jobs.
*/
let lintFiles;
let htmlFixtureFiles;
let invalidWhitespaceFiles;
let linkCheckFiles;
let lintFiles;
let presubmitFiles;
let prettifyFiles;

Expand Down Expand Up @@ -176,9 +177,9 @@ const targetMatchers = {
return false;
}
return (
linkCheckFiles.includes(file) ||
file == 'build-system/tasks/check-links.js' ||
file.startsWith('build-system/tasks/markdown-toc/') ||
(path.extname(file) == '.md' && !file.startsWith('examples/'))
file.startsWith('build-system/tasks/markdown-toc/')
);
},
[Targets.E2E_TEST]: (file) => {
Expand Down Expand Up @@ -330,9 +331,10 @@ function determineBuildTargets() {
return buildTargets;
}
buildTargets = new Set();
lintFiles = globby.sync(config.lintGlobs);
htmlFixtureFiles = globby.sync(config.htmlFixtureGlobs);
invalidWhitespaceFiles = globby.sync(config.invalidWhitespaceGlobs);
linkCheckFiles = globby.sync(config.linkCheckGlobs);
lintFiles = globby.sync(config.lintGlobs);
presubmitFiles = globby.sync(config.presubmitGlobs);
prettifyFiles = globby.sync(config.prettifyGlobs);
const filesChanged = gitDiffNameOnlyMain();
Expand Down
5 changes: 3 additions & 2 deletions build-system/test-configs/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -173,10 +173,11 @@ const prettifyGlobs = [
];

/**
* List of markdown files that may be checked by `amp check-links` (using
* markdown-link-check).
* List of markdown and yaml files that may be checked by `amp check-links`
* (using markdown-link-check).
*/
const linkCheckGlobs = [
'.github/ISSUE_TEMPLATE/*.yml',
'**/*.md',
'!**/{examples,node_modules,build,dist,dist.3p,dist.tools}/**',
];
Expand Down
2 changes: 1 addition & 1 deletion docs/amp-module-build.md
Original file line number Diff line number Diff line change
Expand Up @@ -75,4 +75,4 @@ We Also remove the following polyfills:

## Report a bug

[File an issue](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=Type%3A+Bug&template=bug-report.md&title=) if you find a bug in AMP. Provide a detailed description and steps for reproducing the bug; screenshots and working examples that demonstrate the problem are appreciated!
[File an issue](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=Type%3A+Bug&template=bug-report.yml) if you find a bug in AMP. Provide a detailed description and steps for reproducing the bug; screenshots and working examples that demonstrate the problem are appreciated!
8 changes: 4 additions & 4 deletions docs/contributing-code.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ Significant changes (e.g. new components or significant changes to behavior) req

- [ ] _Before you start coding_, [find a guide](#find-a-guide) who you can discuss your change with and who can help guide you through the process.
- [ ] Agree to the [OpenJSF Contributor License Agreement (CLA)](#contributor-license-agreement).
- [ ] File an [Intent-to-implement (I2I)](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=INTENT+TO+IMPLEMENT&template=intent-to-implement--i2i-.md&title=I2I:%20%3Cyour%20change/update%3E) GitHub issue and cc your guide on it. The I2I should include:
- [ ] File an [Intent-to-implement (I2I)](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=INTENT+TO+IMPLEMENT&template=intent-to-implement.yml) GitHub issue and cc your guide 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.
Expand All @@ -43,7 +43,7 @@ Significant changes (e.g. new components or significant changes to behavior) req
- Familiarize yourself with our [Design Principles](design-principles.md).
- Your guide can help you determine if your change requires a design doc and whether it should be brought to a [design review](./design-reviews.md).
- [ ] Proceed with the [implementation](#implementation) of your change.
- [ ] For changes that require approval from the Approvers WG, file an [Intent-to-ship (I2S) issue](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=INTENT+TO+SHIP&template=intent-to-ship--i2s-.md&title=I2S:%20%3Cyour%20change/update%3E). 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.
- [ ] For changes that require approval from the Approvers WG, file an [Intent-to-ship (I2S) issue](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=INTENT+TO+SHIP&template=intent-to-ship.yml). 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 guide

Expand Down Expand Up @@ -194,9 +194,9 @@ We have a well-defined process for handling requests for changes to the **experi

The following outlines the process to request a cherry-pick.

- Ensure there is a [GitHub issue](https://github.com/ampproject/amphtml/issues/new/choose) filed for the problem that needs to be resolved _before_ filing the cherry-pick request issue.
- Ensure there is a [bug report](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=Type%3A+Bug&template=bug-report.yml) filed for the problem, and ensure it is resolved _before_ filing the cherry-pick request.
- Escalate the issue to P0 by attaining consensus from one or more members of the [Approvers Working Group (WG)](https://github.com/ampproject/wg-approvers) (if you are a member of this working group, you may not count your voice for consensus purposes)
- File the cherry-pick request issue using the [Cherry-pick request template](https://github.com/ampproject/amphtml/issues/new?title=%F0%9F%8C%B8%20Cherry%20pick%20request%20for%20%3CYOUR_ISSUE_NUMBER%3E%20into%20%3CRELEASE_ISSUE_NUMBER%3E%20%28Pending%29&template=cherry_pick_template.md). Follow the instructions in the template, providing all the requested data. **Make sure you fill out this issue completely or your cherry-pick may not be seen or acted upon.**
- File the cherry-pick request issue using the [Cherry-pick request template](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=Type:+Release,Cherry-pick:+Beta,Cherry-pick:+Experimental,Cherry-pick:+LTS,Cherry-pick:+Stable&template=cherry-pick-request.yml&title=%F0%9F%8C%B8+Cherry-pick+request+for+%23ISSUE+into+%23RELEASE). Follow the instructions in the template, providing all the requested data. **Make sure you fill out this issue completely or your cherry-pick may not be seen or acted upon.**
- Get the necessary approval for your cherry-pick, indicated via comments on the cherry-pick request issue.
- For cherry-picks into **experimental**/**beta**, at least one member of the [Approvers WG](https://github.com/orgs/ampproject/teams/wg-approvers/members) must approve the cherry-pick/rollback.
- For cherry-picks into **stable**/**lts** at least one member from the [Cherry-Pick Approvers group](https://github.com/orgs/ampproject/teams/cherry-pick-approvers/members) must approve the cherry-pick.
Expand Down
8 changes: 4 additions & 4 deletions docs/contributing.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,25 @@ How would you like to help?
- [Get started with open source](#get-started-with-open-source)
- [Ongoing participation](#ongoing-participation)

If you have questions about _using_ AMP or are _encountering problems using AMP_ on your site please visit our [support page](SUPPORT.md) for help.
If you have questions about _using_ AMP or are _encountering problems using AMP_ on your site please visit our [support page](./support.md) for help.

## Report a bug

[File an issue](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=Type%3A+Bug&template=bug-report.md&title=) if you find a bug in AMP. Provide a detailed description and steps for reproducing the bug; screenshots and working examples that demonstrate the problem are appreciated!
[File a bug report](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=Type%3A+Bug&template=bug-report.yml) if you find a bug in AMP. Provide a detailed description and steps for reproducing the bug; screenshots and working examples that demonstrate the problem are appreciated!

The community regularly monitoring issues and will try to fix open bugs quickly according to our [prioritization guidelines](./issue-priorities.md).

## Make a suggestion

[File a "feature request" issue](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=Type%3A+Feature+Request&template=feature_request.md&title=) if you have a suggestion for a way to improve AMP or a request for a new AMP feature.
[File a feature request](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=Type%3A+Feature+Request&template=feature-request.yml) if you have a suggestion for a way to improve AMP or a request for a new AMP feature.

If you are suggesting a feature that you are intending to implement, please see the [Contributing code/features](#contribute-code-features) section below for next steps.

## Contribute code/features

We'd love to have your help contributing code and features to AMP!

See the [Contributing code and features](docs/contributing-code.md) document for details on the process you can use to add code/features.
See the [Contributing code and features](./contributing-code.md) document for details on the process you can use to add code/features.

## Get started with open source

Expand Down
2 changes: 1 addition & 1 deletion docs/developing.md
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ To opt your browser into the a pre-release channel, go to [the AMP experiments p

**If you find an issue that appears to only occur in the _Experimental/Beta Channel_ version of AMP**:

- please [file an issue](https://github.com/ampproject/amphtml/issues/new) with a description of the problem
- please [file a bug report](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=Type%3A+Bug&template=bug-report.yml) with a description of the problem
- include a note that the problem is new to the _Experimental/Beta Channel_ build so that it can be properly prioritized
- include a URL to a page that reproduces the problem
- ping the [AMP Slack #release channel](https://amphtml.slack.com/messages/C4NVAR0H3/) ([sign up for Slack](https://bit.ly/amp-slack-signup)) with the issue you filed so we can delay the push of the _Experimental/Beta Channel_ version to production if needed
Expand Down
12 changes: 6 additions & 6 deletions docs/spec/amp-3p-video.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ The generic [`amp-video`](https://go.amp.dev/c/amp-video) component plays videos
Unfortunately, not all videos can be embedded this way. For these specialized cases, AMP provides vendor-optimized components like [`amp-youtube`](https://go.amp.dev/c/amp-youtube), [`amp-ima-video`](https://go.amp.dev/c/amp-ima-video), and [others](./amp-video-interface.md).
Internally these players load an iframe whose page communicates with the outer document to coordinate playback.

**We would prefer to not have additional custom video implementations.** Instead we believe, [`amp-video-iframe`](https://go.amp.dev/c/amp-video-iframe) should be used instead, since it is a generic component that can load any video document to coordinate playback. Instead, new integrations should come in the form of [`amp-video-iframe` configurations](https://go.amp.dev/c/amp-video-iframe#vendors). _If you believe you're unable to integrate with `amp-video-iframe`, please [file an issue](https://github.com/ampproject/amphtml/issues/new/choose) mentioning `@alanorozco` and `@wassgha`._
**We would prefer to not have additional custom video implementations.** Instead we believe, [`amp-video-iframe`](https://go.amp.dev/c/amp-video-iframe) should be used instead, since it is a generic component that can load any video document to coordinate playback. Instead, new integrations should come in the form of [`amp-video-iframe` configurations](https://go.amp.dev/c/amp-video-iframe#vendors). _If you believe you're unable to integrate with `amp-video-iframe`, please [file a bug report](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=Type%3A+Bug&template=bug-report.yml) mentioning `@alanorozco` and `@wassgha`._

## I want to contribute my vendor-specific player

Expand Down Expand Up @@ -52,15 +52,15 @@ For example, these are some of the features that justify specific players:

### Understand the contribution process

You'll need to go through [the standard AMP contribution process](../docs/contributing.md) when creating and submitting your component. For large features, such as a video player, you need to file an [I2I (intent-to-implement) issue](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=INTENT+TO+IMPLEMENT&template=intent-to-implement--i2i-.md&title=I2I:%20%3Cyour%20change/update%3E) that describes the overall workings of your component.
You'll need to go through [the standard AMP contribution process](../contributing.md) when creating and submitting your component. For large features, such as a video player, you need to file an [I2I (intent-to-implement) issue](https://github.com/ampproject/amphtml/issues/new?assignees=&labels=INTENT+TO+IMPLEMENT&template=intent-to-implement.yml) that describes the overall workings of your component.

Your player component is also shipped as an extension, so you should [become familiar with the process of building one.](https://github.com/ampproject/amphtml/blob/main/docs/building-an-amp-extension.md)

### Write your implementation

#### The `VideoManager`

Every player component talks to a single [`VideoManager`](../src/service/video-manager-impl.js) via standard methods (`VideoInterface`) and events (`VideoEvents`) defined in the [AMP video interface](../src/video-interface.js).
Every player component talks to a single [`VideoManager`](../../src/service/video-manager-impl.js) via standard methods (`VideoInterface`) and events (`VideoEvents`) defined in the [AMP video interface](../../src/video-interface.js).

This manager performs standard responsibilities for all videos, regardless of type:

Expand All @@ -72,14 +72,14 @@ This manager performs standard responsibilities for all videos, regardless of ty

#### Interface support

Component classes should implement the [`VideoInterface`](../src/video-interface.js).
Component classes should implement the [`VideoInterface`](../../src/video-interface.js).
You can implement this interface entirely or partially, depending on [the video integration features you'd like to support](./amp-video-interface.md).

At the very least, you should implement `play()` and `pause()`. Likewise, playback
[actions](https://amp.dev/documentation/guides-and-tutorials/learn/amp-actions-and-events/) (like `my-element.play`) will not work when unimplemented.

Read through [the `VideoInterface` code to understand the individual effects of leaving each method unimplemented.](../src/video-interface.js)
Read through [the `VideoInterface` code to understand the individual effects of leaving each method unimplemented.](../../src/video-interface.js)

#### Reference

Existing implementations for [`amp-youtube.js`](../extensions/amp-youtube/0.1/amp-youtube.js) and [`amp-video-iframe.js`](../extensions/amp-video-iframe/0.1/amp-video-iframe.js) are good starter examples for implementation details. They use several standard utilities for creating the video frame and communicating with the `VideoManager`.
Existing implementations for [`amp-youtube.js`](../../extensions/amp-youtube/0.1/amp-youtube.js) and [`amp-video-iframe.js`](../../extensions/amp-video-iframe/0.1/amp-video-iframe.js) are good starter examples for implementation details. They use several standard utilities for creating the video frame and communicating with the `VideoManager`.
Loading