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

Add a PR template and issue template #394

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

OussamaSaoudi-db
Copy link
Collaborator

@OussamaSaoudi-db OussamaSaoudi-db commented Oct 14, 2024

What changes were proposed in this pull request?

This PR adds a pull request template and issues template to the repository.
The issues template is adapted from Apache Datafusion's issue template: https://github.com/apache/datafusion/.
This pull request template is adapted from Apache Spark's pull request template: https://github.com/apache/spark

Why are the changes needed?

A pull request template encourages open source contributions to clearly state their purpose and approach. It explicitly calls out user facing changes and explains why they are needed. It also encourages all contributions to be well tested. In the event testing was difficult or impossible, this exposes testing pain points can be addressed.

Issues template encourages community members and users to give all the details that would help bugs get fixed and feature requests implemented. Requiring more details from issues also helps newcomers to the project get more context to get started contributing.

Does this PR introduce any user-facing change?

No

How was this change tested?

No way to test pull request templates or issue templates.

Copy link

codecov bot commented Oct 14, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 77.66%. Comparing base (9a63e11) to head (22ea47e).
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #394      +/-   ##
==========================================
- Coverage   77.69%   77.66%   -0.03%     
==========================================
  Files          49       49              
  Lines       10087    10076      -11     
  Branches    10087    10076      -11     
==========================================
- Hits         7837     7826      -11     
  Misses       1805     1805              
  Partials      445      445              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@OussamaSaoudi-db OussamaSaoudi-db marked this pull request as ready for review October 14, 2024 21:24
@OussamaSaoudi-db OussamaSaoudi-db changed the title Add a PR template to state rationale, breaking changes, and testing Add a PR template and issue template Oct 14, 2024
Copy link
Collaborator

@nicklan nicklan left a comment

Choose a reason for hiding this comment

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

Thanks. Just a few wording suggestions.

.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved


<!--
Adapted from Apache Spark's pull request template: https://github.com/apache/spark
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we need this?

Copy link
Collaborator

Choose a reason for hiding this comment

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

i think this was originally my suggestion :) but i meant for us to just credit them in the PR description or something - don't need to have it in the actual template

.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved
Copy link
Collaborator

@zachschuermann zachschuermann left a comment

Choose a reason for hiding this comment

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

just lots of wording. but generally looks good - thanks! I'm mostly advocating for making it simpler (I think there is a balance between not enough/too much structure for these things)



<!--
Adapted from Apache Spark's pull request template: https://github.com/apache/spark
Copy link
Collaborator

Choose a reason for hiding this comment

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

i think this was originally my suggestion :) but i meant for us to just credit them in the PR description or something - don't need to have it in the actual template

.github/ISSUE_TEMPLATE/bug_report.yml Outdated Show resolved Hide resolved
.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
A clear and concise description of what you want to happen.
- type: textarea
attributes:
label: Describe alternatives you've considered
Copy link
Collaborator

Choose a reason for hiding this comment

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

overkill?

.github/ISSUE_TEMPLATE/feature_request.yml Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
Copy link
Collaborator

Choose a reason for hiding this comment

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

move 6. to below?

.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
.github/pull_request_template.md Outdated Show resolved Hide resolved
@zachschuermann
Copy link
Collaborator

Also can we make the 'user-facing changes' section of the PR description hidden by default with a comment that says 'uncomment if user-facing changes'? that way we don't just have tons of PRs with 'user-facing changes?' 'No.'

Co-authored-by: Nick Lanham <nicklan@users.noreply.github.com>
Co-authored-by: Zach Schuermann <zachary.zvs@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

4 participants