docs: add PR template, YAML issue forms, and CONTRIBUTING.md#85
Conversation
Closes dgenio#41 - Add .github/pull_request_template.md - Add .github/ISSUE_TEMPLATE/bug_report.yml - Add .github/ISSUE_TEMPLATE/feature_request.yml - Add CONTRIBUTING.md at root
There was a problem hiding this comment.
Pull request overview
Adds GitHub contribution scaffolding (PR template, issue forms) and a contributor guide to standardize how changes/requests are submitted, per #41.
Changes:
- Added a root
CONTRIBUTING.mddescribing setup, workflow, and contribution guidelines - Added a PR template with summary/changes/testing/checklist sections
- Added GitHub Issue Forms for bug reports and feature requests
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
CONTRIBUTING.md |
New contributor onboarding/workflow guide (setup, style, testing, PR process). |
.github/pull_request_template.md |
New PR template to standardize PR metadata and pre-merge checks. |
.github/ISSUE_TEMPLATE/bug_report.yml |
New structured bug report issue form (repro + version fields). |
.github/ISSUE_TEMPLATE/feature_request.yml |
New structured feature request issue form. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| - [ ] All existing tests pass (`pytest`) | ||
| - [ ] New tests added for new functionality | ||
| - [ ] Linting passes (`ruff check`) | ||
| - [ ] Type checking passes (`mypy`) |
There was a problem hiding this comment.
The template asks for mypy to pass, but mypy is not part of the repo's .[dev] dependencies, and CI doesn't run it. Either add mypy to the dev extra/CI, or remove/soften this checklist item so contributors aren't required to run a tool that isn't provided by the project.
| - [ ] Type checking passes (`mypy`) | |
| - [ ] Type checking passes (e.g., `mypy`, if configured) |
|
|
||
| - [ ] All existing tests pass (`pytest`) | ||
| - [ ] New tests added for new functionality | ||
| - [ ] Linting passes (`ruff check`) |
There was a problem hiding this comment.
CI also checks formatting with ruff format --check in addition to ruff check. Consider adding an explicit formatting checklist item (and/or clarifying the exact command/paths) so the PR template matches the checks contributors will see in CI.
| - [ ] Linting passes (`ruff check`) | |
| - [ ] Linting passes (`ruff check`) | |
| - [ ] Formatting passes (`ruff format --check`) |
| name: Bug Report | ||
| description: Report a bug in ChainWeaver | ||
| title: "[Bug]: " | ||
| labels: ["bug"] | ||
| body: |
There was a problem hiding this comment.
| - type: textarea | ||
| id: solution | ||
| attributes: | ||
| label: Proposed Solution | ||
| description: Describe the solution you'd like. | ||
| validations: | ||
| required: true | ||
|
|
There was a problem hiding this comment.
Issue #41 specifies the feature request form should have an optional proposed-solution field, but this form currently makes Proposed Solution required. Consider making it optional so reporters can submit a request even when they don't have a concrete solution yet.
|
|
||
| - Follow existing project conventions (see `.github/copilot-instructions.md`) | ||
| - Use [ruff](https://github.com/astral-sh/ruff) for linting: `ruff check .` | ||
| - Use [mypy](https://mypy-lang.org/) for type checking: `mypy .` |
There was a problem hiding this comment.
mypy is listed as a required/expected type-checking tool, but it is not included in the project's dev extra in pyproject.toml. Either add mypy to [project.optional-dependencies].dev (and ideally CI) or adjust this guidance so contributors following pip install -e ".[dev]" aren't blocked by a missing tool.
| - Use [mypy](https://mypy-lang.org/) for type checking: `mypy .` | |
| - (Optional but recommended) Use [mypy](https://mypy-lang.org/) for type checking: `mypy .` (install with `pip install mypy` if it is not already available) |
| ### Code Style | ||
|
|
||
| - Follow existing project conventions (see `.github/copilot-instructions.md`) | ||
| - Use [ruff](https://github.com/astral-sh/ruff) for linting: `ruff check .` |
There was a problem hiding this comment.
CI runs Ruff on specific paths and also enforces formatting with ruff format --check (see .github/workflows/ci.yml). Consider updating these commands to match CI (e.g., include formatting check and align the lint target paths) so contributors can reproduce CI results locally.
| - Use [ruff](https://github.com/astral-sh/ruff) for linting: `ruff check .` | |
| - Use [ruff](https://github.com/astral-sh/ruff) for linting and formatting (mirroring the CI configuration in `.github/workflows/ci.yml`): | |
| ```bash | |
| ruff check src tests | |
| ruff format --check src tests |
|
|
||
| - Run the test suite before submitting: | ||
| ```bash | ||
| pytest |
There was a problem hiding this comment.
Local test instructions use pytest, while CI runs python -m pytest tests/ -v. Since this guide is meant to help contributors preflight CI, consider matching the CI invocation (including tests/ and -v) to reduce surprises.
| pytest | |
| python -m pytest tests/ -v |
dgenio
left a comment
There was a problem hiding this comment.
Good docs-only PR — the templates and CONTRIBUTING.md are well-structured. One item needs to be addressed before merge:
- Missing
config.yml— Issue #41's definition-of-done requires.github/ISSUE_TEMPLATE/config.ymlwithblank_issues_enabled: true. Since this PR says "Closes #41," it should complete the full spec.
Additionally (non-blocking):
feature_request.yml: consider splitting "Problem Statement" into separate "Description" and "Use case" fields, and marking "Proposed Solution" as optional, per the issue spec.CONTRIBUTING.md: references.github/copilot-instructions.mdwhich doesn't exist yet (issue #53), and uses bareruff check ./mypy .instead of the project's scoped paths.
| - type: textarea | ||
| id: problem | ||
| attributes: | ||
| label: Problem Statement | ||
| description: What problem does this feature solve? | ||
| validations: | ||
| required: true | ||
|
|
||
| - type: textarea | ||
| id: solution | ||
| attributes: | ||
| label: Proposed Solution | ||
| description: Describe the solution you'd like. | ||
| validations: | ||
| required: true |
There was a problem hiding this comment.
Issue #41 specifies separate "Description" and "Use case" fields. This collapses them into "Problem Statement" (L9). Also "Proposed Solution" is required: true (L20) but the spec says optional. Consider aligning with the issue spec.
| - Use [ruff](https://github.com/astral-sh/ruff) for linting: `ruff check .` | ||
| - Use [mypy](https://mypy-lang.org/) for type checking: `mypy .` | ||
|
|
There was a problem hiding this comment.
Two minor items:
- L28:
.github/copilot-instructions.mdis referenced but doesn't exist yet (issue Add .github/copilot-instructions.md with coding conventions and validation commands #53). Consider noting it's forthcoming or removing for now. - L29–L30:
ruff check .andmypy .— project convention uses scoped paths (ruff check chainweaver/ tests/ examples/,mypy chainweaver/).
Summary
Adds structured templates and contributor guide as described in #41.
Changes
.github/pull_request_template.mdwith testing checklist and conventions.github/ISSUE_TEMPLATE/bug_report.yml(YAML form with Python/ChainWeaver version fields).github/ISSUE_TEMPLATE/feature_request.yml(YAML form)CONTRIBUTING.mdwith setup, workflow, commit, and PR guidelinesTesting
Related Issues
Closes #41
Checklist