Skip to content
This repository has been archived by the owner on May 3, 2024. It is now read-only.

chore(template): add pull request template #43

Merged
merged 11 commits into from
Mar 4, 2020
38 changes: 38 additions & 0 deletions .github/pull_request_template.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
<!--- Provide a general summary of your changes in the Title above -->

## Description
<!--- Describe your changes in detail -->

## Motivation and Context
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->

## How Has This Been Tested?
<!--- Please describe in detail how you tested your changes. -->
<!--- Include details of your testing environment, tests ran to see how -->
<!--- your change affects other areas of the code, etc. -->

## Types of Changes
<!--- What types of changes does your code introduce? Put an `x` in all the boxes that apply: -->
- [ ] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected)
- [ ] Documentation (adding or updating documentation)

## Checklist:
JAdshead marked this conversation as resolved.
Show resolved Hide resolved
<!--- Go over all the following points, and put an `x` in all the boxes that apply. -->
<!--- If you're unsure about any of these, don't hesitate to ask. We're here to help! -->
- [ ] My code follows the code style of this project.
JamesSingleton marked this conversation as resolved.
Show resolved Hide resolved
- [ ] My change requires a change to the documentation.
- [ ] I have updated the documentation accordingly.
JamesSingleton marked this conversation as resolved.
Show resolved Hide resolved
- [ ] These changes should be applied to a maintenance branch.
- [ ] This change requires cross browser checks.
- [ ] This change requires a performance test prior to merging.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should specify if you mean client side of server side perf test.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do we mean in the current Preflight Checklist?

- [ ] This change impacts caching.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should specify that you mean caching for client browsers.

- [ ] This change impacts HTTP headers.
- [ ] This change has new infrastructure requirements.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be explained in more detail

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it should. Any input here as it was a question in the preflight checklist and wasn't sure.

- [ ] This change requires additional environment variables.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about This change adds additional environment variable requirements for One App users?

- [ ] I have added the Apache 2.0 license to any new files created.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be license header.


## What is the Impact to Developers Using the App?
<!--- Please describe how your changes impacts developers using the app. -->
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/app/One App