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

Update PR Template #4754

Merged
merged 1 commit into from
Jan 20, 2023
Merged

Update PR Template #4754

merged 1 commit into from
Jan 20, 2023

Conversation

stnguyen90
Copy link
Contributor

What does this PR do?

Update PR Template to:

  1. Ask for why in description to help with troubleshooting in the future
  2. Change related PRs to a list so it renders nicer
  3. Remove changes.md task in favor of just using milestones to track changes
  4. Add reminder to check if API specs need to be updated

Test Plan

None

Related PRs and Issues

None

Have you added your change to the Changelog?

(The CHANGES.md file tracks all the changes that make it to the main branch. Add your change to this file in the following format)

  • One line description of your PR [#pr_number](Link to your PR)

Have you read the Contributing Guidelines on issues?

Yes

1. Ask for why in description to help with troubleshooting in the future
2. Change related PRs to a list so it renders nicer
3. Remove changes.md task in favor of just using milestones to track changes
4. Add reminder to check if API specs need to be updated

## Related PRs and Issues

(If this PR is related to any other PR or resolves any issue or related to any issue link all related PR and issues here.)
- (Related PR or issue)

Choose a reason for hiding this comment

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

Suggested change
- (Related PR or issue)
- (Related PRs or issues)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, but I made it singular because each bullet should be 1 issue or PR.

Comment on lines +26 to +27
- [ ] Have you read the [Contributing Guidelines on issues](https://github.com/appwrite/appwrite/blob/master/CONTRIBUTING.md)?
- [ ] If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?

Choose a reason for hiding this comment

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

Suggested change
- [ ] Have you read the [Contributing Guidelines on issues](https://github.com/appwrite/appwrite/blob/master/CONTRIBUTING.md)?
- [ ] If the PR includes a change to an API's metadata (desc, label, params, etc.), does it also include updated API specs and example docs?
- [ ] I have carefully read the [Contributing Guidelines on issues](https://github.com/appwrite/appwrite/blob/master/CONTRIBUTING.md)
- [ ] I have updated the API specs and example docs, when changing the API's metadata (desc, label, params, etc.).

Choose a reason for hiding this comment

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

i would re-write this in order to be more "active" ;)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your suggestion, but, for now, I'd rather keep this closer to the original as that's what's in most of our other repos. I'll keep your suggestion in mind when we do a rehaul of all of our templates.

@@ -11,21 +11,17 @@ Happy contributing!

## What does this PR do?

(Provide a description of what this PR does.)
(Provide a description of what this PR does and why it's needed.)

Choose a reason for hiding this comment

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

Can we follow PR template like this

Objective

-Brief about the PR-

Description

-Describe the change-

How to Test

-Mention the steps to follow to verify the changes-

Screenshots

-Add screenshots-

Checklist

-[ ] Merged with latest branch
-[ ] Verified unit tests
-[ ] Code clean (Remove unnecessary codes)
-[ ] Attached all screenshots (Including FE)

I believe this will work for all the PRs like feature, bug fix. Please share your thoughts on this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the suggestion, but I'd like to limit the scope of the change in this PR. If you feel strongly about your PR template, please submit an issue with info on why we should change to your suggested format.

@christyjacob4 christyjacob4 merged commit 2f252ec into master Jan 20, 2023
@stnguyen90 stnguyen90 deleted the feat-pr-template branch February 14, 2023 00:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants