Skip to content

Add pull request templates - #51

Draft
makhlwf wants to merge 4 commits into
nvaccess:masterfrom
makhlwf:pr-templates
Draft

Add pull request templates#51
makhlwf wants to merge 4 commits into
nvaccess:masterfrom
makhlwf:pr-templates

Conversation

@makhlwf

@makhlwf makhlwf commented Aug 8, 2026

Copy link
Copy Markdown

This adds default PR templates for the add-on template repository, covering part of #48.

Comment thread .github/PULL_REQUEST_TEMPLATE/code_change.md Outdated
Comment thread .github/PULL_REQUEST_TEMPLATE/translation.md Outdated
Comment thread .github/PULL_REQUEST_TEMPLATE.md Outdated
@@ -0,0 +1,33 @@
<!---

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It will probably be easier for most repos and contributors to just use a single template, like we do in NVDA.
I think we do need two different PR templates though, one designed for contributions to this template, and one designed for contributions to add-ons.
when syncing via #46 , we should set the default template appropriately.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

In the latest commits, I reworked the templates, so now we have only two: one for changes to the template itself and the other for the add-ons. I put them in the pull requests directory in the .github folder, so when someone makes a pull request, they will show up together. It's easy for me, after the mentioned pull requests are merged, to make the script delete the template-specific pull request template, then move the add-on up a directory and rename it.

@seanbudd
seanbudd marked this pull request as draft August 10, 2026 01:59
@makhlwf
makhlwf marked this pull request as ready for review August 12, 2026 00:37
@makhlwf

makhlwf commented Aug 12, 2026

Copy link
Copy Markdown
Author

i think this will address all consernes

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

The add-on PR template includes empty YAML front matter fields (title: '', labels: '') that should be removed to avoid unintended behavior or noisy metadata.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds multiple pull request templates under .github/PULL_REQUEST_TEMPLATE/ to standardize PR descriptions for both template-infrastructure changes and downstream add-on changes, aligning with the “default GitHub project files” work in #48.

Changes:

  • Added a template-focused PR template (TEMPLATE_PR.md) for changes to the AddonTemplate repo infrastructure/tooling/docs.
  • Added an add-on-focused PR template (ADDON_PR.md) for feature/fix PRs in downstream NVDA add-on repos created from this template.
File summaries
File Description
.github/PULL_REQUEST_TEMPLATE/TEMPLATE_PR.md Adds a PR template tailored to template infrastructure/build/CI/tooling changes.
.github/PULL_REQUEST_TEMPLATE/ADDON_PR.md Adds a PR template tailored to add-on feature/fix/testing/metadata validation.
Review details
  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment on lines +1 to +6
---
name: NVDA Add-on Feature / Fix PR
about: Use this template for PRs that modify add-on functionality, features, bug fixes, or translations.
title: ''
labels: ''
---
@@ -0,0 +1,36 @@
---

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you please also add a symlink from PULL_REQUEST_TEMPLATE.md to this file, and documentation encouraging add-on repo owners to update the symlink to the other template

Comment on lines +4 to +5
title: '[Template] '
labels: 'template'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

does this work with PR templates? how?

### Migration / Upgrade Instructions (if applicable)
<!-- Detail any steps downstream maintainers must take to adopt these template changes. -->

## Local Quality Checks & Verification

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think this section should be broader: just a "Testing strategy" section to cover how each PR is tested

- [ ] Ran `pytest` with all tests passing.
- [ ] Verified CI workflow script behavior locally or via test workflow run.

## Documentation & Changelog

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

there's no changelog

Comment on lines +35 to +36
## Documentation & Changelog
- [ ] Updated `readme.md` or developer documentation (if applicable).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Suggested change
## Documentation & Changelog
- [ ] Updated `readme.md` or developer documentation (if applicable).

Comment on lines +14 to +21
## Type of Change
- [ ] Build System / SCons updates (`sconstruct`, `site_scons/`)
- [ ] CI/CD & GitHub Actions (`.github/workflows/`, `.github/scripts/`)
- [ ] Development Dependencies & Tooling (`pyproject.toml`, `uv.lock`, `prek.toml`)
- [ ] Template Documentation & Boilerplate (`readme.md`, `docs/`, `manifest.ini.tpl`)
- [ ] Bug fix in template scripts/code
- [ ] Refactoring / Code Quality improvement

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

most of these are just listing files in the diff, which can be checked by looking at the diff

Suggested change
## Type of Change
- [ ] Build System / SCons updates (`sconstruct`, `site_scons/`)
- [ ] CI/CD & GitHub Actions (`.github/workflows/`, `.github/scripts/`)
- [ ] Development Dependencies & Tooling (`pyproject.toml`, `uv.lock`, `prek.toml`)
- [ ] Template Documentation & Boilerplate (`readme.md`, `docs/`, `manifest.ini.tpl`)
- [ ] Bug fix in template scripts/code
- [ ] Refactoring / Code Quality improvement

## NVDA Testing & Verification Environment
- **Minimum NVDA Version Tested:** <!-- e.g. 2024.1 -->
- **Latest NVDA Version Tested:** <!-- e.g. 2026.2 / latest alpha -->
- **OS / Platform:** Windows 10 / 11

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

it might be worth asking for full windows version here including version number


## NVDA Testing & Verification Environment
- **Minimum NVDA Version Tested:** <!-- e.g. 2024.1 -->
- **Latest NVDA Version Tested:** <!-- e.g. 2026.2 / latest alpha -->

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

"latest alpha" is never useful, we always need version numbers

Comment on lines +36 to +39
## Local Quality Checks
- [ ] Ran `ruff check .` / `prek` cleanly.
- [ ] Ran `pytest` with 100% passing tests.
- [ ] Tested add-on bundle installation locally (`scons`).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

similarly I think "Testing strategy" is more useful than "local quality checks". most of these checks would be automated via ci/cd

@seanbudd
seanbudd marked this pull request as draft September 4, 2026 07:19
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.

3 participants