Skip to content

PYTHON-5576: add PR template to mongo-python-driver #2567

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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Oct 7, 2025
Merged

PYTHON-5576: add PR template to mongo-python-driver #2567

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
99ea739
create pull request template (not sure why uv lock changed)
sleepyStick Sep 23, 2025
eb65cc0
change section descriptions to be comments to the PR author
sleepyStick Sep 29, 2025
f9b4215
rename to lowercase?
sleepyStick Sep 30, 2025
2fde260
add issue key to top of template and 'test' -> 'testing'
sleepyStick Oct 2, 2025
a0a547c
remove old template
sleepyStick Oct 2, 2025
55bcc69
add blurb about PR title format and make focus area optional
sleepyStick Oct 2, 2025
f5a727c
remove link to PR template google doc
sleepyStick Oct 6, 2025
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
33 changes: 33 additions & 0 deletions .github/pull_request_template.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<!-- Thanks for contributing! -->
## Summary
<!-- What conceptually is this PR introducing? If context is already provided from the JIRA ticket, still place it in the
Copy link
Contributor

Choose a reason for hiding this comment

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

Is a JIRA ticket required? I think so. This should be included, as should our convention for naming PRs, as well as including a link to our jira project.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added jira link to the top.
I left out the PR naming convention because that's not part of the PR body. Would you like the naming convention to be left as a comment at the top for the PR author? (I feel like most of us already understand the PR naming convention like we use it already?)

Copy link
Contributor

Choose a reason for hiding this comment

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

@sleepyStick Here's some more content that you can use to update this based on our team meeting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry if I'm misunderstanding your most recent comment. But I've gone ahead and added a blurb in the template about PR name.

Copy link
Contributor

Choose a reason for hiding this comment

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

All good.

Pull Request as you should not make the reviewer do digging for a basic summary. -->

## Changes in this PR
<!-- What changes did you make to the code? What new APIs (public or private) were added, removed, or edited to generate
the desired outcome explained in the above summary? -->

## Test Plan
Copy link
Contributor

Choose a reason for hiding this comment

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

Considering changing to "Testing". Plan feels forward looking, while the description below is backward looking.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed!

<!-- How did you test the code? If you added unit tests, you can say that. If you didn’t introduce unit tests, explain why.
All code should be tested in some way – so please list what your validation strategy was. -->

### Screenshots (optional)
<!-- Usually a great supplement to a test plan, especially if this requires local testing. -->

## Checklist
<!-- Do not delete the items provided on this checklist. -->
### Checklist for Author
- [ ] Did you update the changelog (if necessary)?
- [ ] Is the intention of the code captured in relevant tests?
- [ ] If there are new TODOs, has a related JIRA ticket been created?

### Checklist for Reviewer {@primary_reviewer}
- [ ] Does the title of the PR reference a JIRA Ticket?
- [ ] Do you fully understand the implementation? (Would you be comfortable explaining how this code works to someone else?)
- [ ] Have you checked for spelling & grammar errors?
- [ ] Is all relevant documentation (README or docstring) updated?

## Focus Areas for Reviewer
<!-- List any complex portion of code you believe needs additional scrutiny and explain why. -->
Copy link
Contributor

Choose a reason for hiding this comment

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

This section seems a bit redundant. This is what one does when commenting on lines of the PR. Are you suggesting that these complex portions be added to the PR description?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was taken from the the template that Jib had previously shown to us.
I believe the intention of this section isn't to copy/paste code, but to call out certain aspects of the code that require extra attention. For example, for this PR, I wanted the reviews to focus their attention on checking for typos and to comment on any formatting preferences. I wouldn't put that as a comment in the template because it wouldn't make sense for a comment in the template to "look out for typos".
@Jibola did I understand the intention of this section correctly?
Maybe we need to reword the description of this to make it more clear?

Copy link
Contributor

Choose a reason for hiding this comment

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

I understand now. I also like the idea of making it optional. I would leave as-is other than that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Its marked as optional now.

Copy link
Contributor

Choose a reason for hiding this comment

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

@sleepyStick yes, that's the intent of the section, and having it as explicitly optional makes sense as well.


<!-- See also: https://docs.google.com/document/d/1Z-z6BDIBJ9G4fn4MBb7Ql5A1NiSY6nsLfEE3KuU_Btw/edit?tab=t.exaie3tsb7gl#heading=h.asd8fqlsyzb6 -->
Copy link
Contributor

Choose a reason for hiding this comment

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

This is an internal google doc only valuable to the team. I'd remove it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!
I initially added it because I noticed that the spec repo had a link to wiki for writing specs and I thought this was our equivalent.

2 changes: 1 addition & 1 deletion uv.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading