Docs: update contributing.rst and PR template for changelog fragments#5478
Conversation
The fragment-based changelog system landed in isaac-sim#5434. Two contributor- facing references still pointed at the old "edit CHANGELOG.rst directly" workflow: - docs/source/refs/contributing.rst — Maintaining a changelog and extension.toml section described per-version editing of CHANGELOG.rst with manual SemVer bumps. - .github/PULL_REQUEST_TEMPLATE.md — checklist item asked contributors to update the changelog and bump extension.toml. Replaced the directly-affected paragraphs and the sample with the fragment workflow. Section/style guidance (Added/Changed/Deprecated/ Removed/Fixed, past tense, sample bullets) stays verbatim.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
This PR updates contributor-facing documentation to reflect the new fragment-based changelog system introduced in PR #5434. It modifies the PR template checklist and the contributing.rst documentation to instruct contributors to create changelog fragments instead of directly editing CHANGELOG.rst and extension.toml. The changes are purely documentation updates with no code impact.
Architecture Impact
Self-contained. These changes affect only documentation and the PR template — no runtime code, no API changes, no behavioral modifications. The changes align contributor guidance with the already-merged fragment-based changelog CI workflow.
Implementation Verdict
Ship it — Minor documentation improvements only, no issues found.
Test Coverage
Documentation-only PR — no tests required. The author confirmed local Sphinx build succeeds, which is the appropriate validation for documentation changes.
CI Status
No CI checks available yet. For a docs-only PR, the main concern would be Sphinx build success, which the author has verified locally.
Findings
🔵 Improvement: docs/source/refs/contributing.rst:152-165 — Fragment documentation could clarify location more explicitly
The note explains the fragment system well, but new contributors might benefit from an explicit example path. The instruction says source/<package>/changelog.d/<slug>.<tier>.rst but doesn't clarify what <package> values are valid (e.g., isaaclab, isaaclab_tasks, isaaclab_rl, etc.). Consider adding a concrete example like:
For example, if your PR modifies ``source/isaaclab/``, create
``source/isaaclab/changelog.d/my-branch-name.minor.rst``.This is a minor suggestion — the current text is functional.
🔵 Improvement: .github/PULL_REQUEST_TEMPLATE.md:50 — Em dash renders inconsistently across platforms
The checklist item uses — (em dash) which may render as ? or similar in some terminals/editors when viewing raw markdown. Consider using -- for maximum compatibility:
- [ ] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` -- CI handles that)This is extremely minor and the current version renders correctly on GitHub's web UI.
🔵 Improvement: docs/source/refs/contributing.rst:193 — Example filename could match PR conventions
The example source/isaaclab/changelog.d/<slug>.minor.rst is good, but showing a realistic slug (like feature-my-feature.minor.rst or fix-buffer-reset.rst) would make the instruction more concrete for first-time contributors.
Overall Assessment: Clean, well-scoped documentation update. The changes correctly replace outdated instructions with the new fragment-based workflow. The example RST fragment structure (lines 195-227) is preserved and still provides excellent guidance on changelog entry formatting. No blocking issues.
Greptile SummaryThis PR updates the two contributor-facing references that still described the old "edit Confidence Score: 4/5Safe to merge — documentation-only changes with no runtime impact; only P2 style findings remain. Both files contain only documentation text changes; the new instructions are accurate and consistent with the fragment system. Two minor P2 style issues were found (stale section heading, placeholder name mismatch) that do not affect correctness. docs/source/refs/contributing.rst — section heading still references extension.toml after the workflow change. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A([Contributor opens PR]) --> B{Touches source code\nin source/package/?}
B -- Yes --> C[Create fragment file\nsource/package/changelog.d/slug.tier.rst]
B -- No\nCI / docs / tests only --> D[Create source/package/changelog.d/slug.skip]
C --> E{Tier?}
E -- patch --> F[slug.rst]
E -- minor\nnew public API --> G[slug.minor.rst]
E -- major\nbreaking change --> H[slug.major.rst]
F & G & H & D --> I([CI merges fragments\nand bumps extension.toml\nvia Semantic Versioning])
I --> J([CHANGELOG.rst updated\nautomatically])
|
| - [ ] My changes generate no new warnings | ||
| - [ ] I have added tests that prove my fix is effective or that my feature works | ||
| - [ ] I have updated the changelog and the corresponding version in the extension's `config/extension.toml` file | ||
| - [ ] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` — CI handles that) |
There was a problem hiding this comment.
Placeholder name inconsistency with contributing docs
The template uses <pkg> while contributing.rst consistently uses <package> for the same path token (source/<package>/changelog.d/). Aligning the two keeps the contributor-facing docs coherent.
| - [ ] I have added a changelog fragment under `source/<pkg>/changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` — CI handles that) | |
| - [ ] I have added a changelog fragment under `source/<package>/changelog.d/` for every touched package (do **not** edit `CHANGELOG.rst` or bump `extension.toml` — CI handles that) |
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
Follow-up review after new commits. The documentation changes remain the same — updating contributor guidance for the fragment-based changelog system.
Implementation Verdict
Ship it
CI Status
Pre-commit passes ✅, changelog fragment check passes ✅, and the "Check for Broken Links" failure appears unrelated to this PR's changes (likely a pre-existing or transient issue with external URLs). The "Build Latest Docs" job is still pending but should be monitored before merge to confirm the RST renders correctly.
Findings
Previous suggestions about clarifying package paths, em dash rendering, and example slugs were minor improvements, not blockers — the current documentation is clear and functional. No new issues introduced.
The PR is ready to merge once the docs build completes successfully.
There was a problem hiding this comment.
🤖 Isaac Lab Review Bot
Summary
Follow-up review after commit cfa7e2f. No code changes since the previous review — the diff remains identical to what was already reviewed.
Implementation Verdict
Ship it
Findings
No new changes to review. The documentation updates for the fragment-based changelog system are correct and ready to merge. The "Check for Broken Links" CI failure appears unrelated to this PR (no links were modified). Monitor the "Build Latest Docs" job to confirm RST renders correctly before merging.
Follow-up to #5434 (fragment-based changelog system). Two contributor-facing references still pointed at the old "edit CHANGELOG.rst directly" workflow:
docs/source/refs/contributing.rst— Maintaining a changelog and extension.toml section described per-version editing of CHANGELOG.rst with manual SemVer bumps..github/PULL_REQUEST_TEMPLATE.md— checklist asked contributors to update the changelog and bump extension.toml directly.Replaced only the parts that talk about direct editing; section/style guidance (Added/Changed/Deprecated/Removed/Fixed, past tense, the sample bullets themselves) stays intact.
Test plan
cc @kellyguo11 — addresses the doc gaps flagged after #5434 merged.