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

ENH: Add PR template #628

Merged
merged 3 commits into from
May 30, 2024
Merged

ENH: Add PR template #628

merged 3 commits into from
May 30, 2024

Conversation

richford
Copy link
Contributor

@richford richford commented May 30, 2024

Proposed changes

This PR adds a PR template that will serve as a starting point for all future PRs. It is intended to be a general purpose PR template for use in ALL ROAR repositories. If it is approved and merged for this repository, I will then add it to all other ROAR repos.

Types of changes

What types of changes does your code introduce?
Put an x in the boxes that apply

  • Bugfix (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 Update (if none of the other choices apply)

Checklist

Put an x in the boxes that apply. You can also fill these out after creating the PR. If you're unsure about any of them, don't hesitate to ask. We're here to help! This is simply a reminder of what we are going to look for before merging your code.

  • I have read the guidelines for contributing.
  • The changes in this PR are as small as they can be. They represent one and only one fix or enhancement.
  • Linting checks pass with my changes.
  • Any existing unit tests pass with my changes.
  • Any existing end-to-end tests pass with my changes.
  • I have added tests that prove my fix is effective or that my feature works.
  • If this PR fixes an existing issue, I have added a unit or end-to-end test that will detect if this issue reoccurs.
  • I have added JSDoc comments as appropriate.
  • I have added the necessary documentation to the roar-docs repository.

Justification of missing checklist items

This PR does not warrant adding any tests.

Copy link

github-actions bot commented May 30, 2024

Visit the preview URL for this PR (updated for commit 9208b89):

https://roar-staging--pr628-enh-add-pr-template-mtnr9rvr.web.app

(expires Thu, 06 Jun 2024 21:11:32 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: 2631e9c58fd0104ecbfddd72a62245ddac467460

@Zio-4
Copy link
Contributor

Zio-4 commented May 30, 2024

I think we should add a section for images if applicable.

Copy link

cypress bot commented May 30, 2024

Passing run #2210 ↗︎

0 26 0 0 Flakiness 0

Details:

Tests for PR 628 "ENH: Add PR template" from commit "9208b893473bf4e3c616f2b4d7c...
Project: roar-dashboard-e2e Commit: 9208b89347
Status: Passed Duration: 03:28 💡
Started: May 30, 2024 9:10 PM Ended: May 30, 2024 9:14 PM

Review all test suite changes for PR #628 ↗︎

Copy link
Collaborator

@lucasxsong lucasxsong left a comment

Choose a reason for hiding this comment

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

General approval, and happy to have this. Think the template will give PR reviewing a lot of useful context and provide good documentation on new additions as well. Just had two small comments. Thanks for writing this up!

.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
.github/PULL_REQUEST_TEMPLATE.md Outdated Show resolved Hide resolved
@richford
Copy link
Contributor Author

I think we should add a section for images if applicable.

Resolved in latest commit by adding a suggestion for images in the HTML comment for the first section.

@richford richford merged commit c894424 into main May 30, 2024
15 of 16 checks passed
@richford richford deleted the enh/add-pr-template branch May 30, 2024 21:18
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.

4 participants