Skip to content

Update review skills and changelogs to use unreleased section#2672

Merged
PawelPeczek-Roboflow merged 4 commits into
mainfrom
codex/unreleased-version-handling
Jul 20, 2026
Merged

Update review skills and changelogs to use unreleased section#2672
PawelPeczek-Roboflow merged 4 commits into
mainfrom
codex/unreleased-version-handling

Conversation

@dkosowski87

Copy link
Copy Markdown
Contributor

What does this PR do?

  • Changes the release process to use Unreleased section in CHANGELOGs for contributors to fill
  • Changes the review skill to require that way of working, plus notifying maintainers about the needed version file changes.

Type of Change

  • New feature (non-breaking change that adds functionality)

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code where necessary, particularly in hard-to-understand areas
  • My changes generate no new warnings or errors
  • I have updated the documentation accordingly (if applicable)

@github-actions

Copy link
Copy Markdown
Contributor

👋 Thanks for the pull request! Here is how automated Claude review works here, so you spend credits (and reviewer time) wisely.

🚦 This PR is marked Ready for review, so automated Claude review will run — and every pass spends real credits.

Warning

💸 The Claude reviewer bills in credits, not vibes

Automated review spins up a real agent that reads real code and spends real credits on every pass. It is glad to help — but it is not a rubber duck, a linter you poke in a loop, or a substitute for reading the contributing guide. Treat it like an expensive senior reviewer whose time you booked, and show up prepared.

Draft when unsure, Ready when you mean it:

  • 🌱 Not sure the PR is in good shape yet? Keep it (or set it back) as a draft — drafts pause review, so you can push and iterate without burning credits on a moving target.
  • 💪 Feel strong about the contents? Mark it Ready for review and the reviewer will take a look.

However you get there, arrive prepared:

  • 🧱 Bring a SOLID, thorough PR. Point your local agent at our skills/ to tune it to our guidelines first — or, if you are one of those fabled carbon-based contributors, read them yourself. A half-baked diff costs exactly the same to review as a finished one.
  • Resolve every comment before you re-request review. Re-requesting with threads still open means paying twice for the same conversation.
  • 🔁 Do not use CI review as an inner loop for a local agent. The reviewer is not a step-by-step debugger — do the unfolding locally and arrive with the answer, not the search.
  • 🙋 If something looks off, ask a human. One question to a maintainer is cheaper and faster than three rounds of agent re-review chasing a misread.

Reviews are not free. A draft costs nothing to review; a Ready PR is a promise that it is worth reviewing.

  • Prefer to skip automated review entirely? Add the skip-claude-review label.

@github-actions

Copy link
Copy Markdown
Contributor

🤖 Claude review started at commit 36068a45d5a4ac26add46114f42aa8ce74a55bd4.

New commits are not auto-reviewed. Add the claude-review label (remove & re-add it to trigger again) when you want another review.

- **Tests** — unit test always; integration test for new or output-changing blocks (#2362, #718).
- **`__init__.py`** — in every new/nested block package (#2351, #830).
- **EE version + changelog** — ONLY when the block forces an EE behavior change: bump `EXECUTION_ENGINE_V1_VERSION` (`execution_engine/v1/core.py`) + entry in `docs/workflows/execution_engine_changelog.md` (#2527).
- **EE changelog** — ONLY when the block forces an EE behavior change: add an entry under `## Unreleased` in `docs/workflows/execution_engine_changelog.md`; maintainers bump `EXECUTION_ENGINE_V1_VERSION` at release time.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Medium — incomplete migration leaves contradictory version-bump guidance in this same skill.

This PR updated this "Required companions" bullet to the Unreleased model (contributor adds a ## Unreleased entry; maintainers bump EXECUTION_ENGINE_V1_VERSION at release time). But two other lines in this same file were left on the old policy and now directly contradict it:

  • Line 24 (7. **FLAG — EE compatibility**): "If the PR bumps EXECUTION_ENGINE_V1_VERSION, require a changelog entry + version-assert test updates …" — still tells the reviewer to require contributor version-assert test updates.
  • Line 49 (Standards, **EE compatibility**): "Only bump it (+ changelog + version-assert tests) when the block forces an EE behavior change (Revert "Add tracklet recognition workflow blocks" #2527 …)" — still instructs the contributor to bump the EE version and update version-assert tests.

Both conflict with the new line 62 here and with the sibling review-workflows-execution-engine skill (updated in this PR to "Never demand an EXECUTION_ENGINE_V1_VERSION … bump from a contributor" and "Version-assertion tests change only in the maintainer release PR") and review-topic-backward-compat-and-versioning §3. A reviewer loading review-workflows-blocks on a block PR that forces an EE change would get conflicting guidance and could still demand a contributor version bump — exactly what this PR aims to eliminate.

Please update lines 24 and 49 to the Unreleased model (contributor adds the ## Unreleased entry; maintainers bump the EE version + version-assert tests at release).

@github-actions

Copy link
Copy Markdown
Contributor

Review summary

This PR migrates the review skills, the cursor rule, the review prompt, and the two changelog docs from a contributor-owned version bump model to an Unreleased-section model (contributors record changes under ## Unreleased; maintainers select and bump versions at release time). The changelog additions are docs-only and don't feed any runtime/test parsing, so they're safe. The bulk of the migration is internally consistent across the skills.

One blocking-quality finding (Medium):

  • review-workflows-blocks/SKILL.md — the migration was applied to the "Required companions" bullet (line 62) but not to lines 24 and 49 of the same file, which still instruct the reviewer to require a contributor EXECUTION_ENGINE_V1_VERSION bump + version-assert test updates. Those now contradict this PR's own change and the sibling review-workflows-execution-engine / review-topic-backward-compat-and-versioning updates. Details inline. This should be fixed so the skill doesn't re-introduce the demand this PR is removing.

No other blocking findings. No IMPORTANT clarification questions are open.

Commands used: gh pr diff, gh api .../commits|comments|reviews, Grep/Read over .claude/skills/** (note: the checkout restores .claude/skills from the base branch, so the PR's skill changes were reviewed via the diff, not the working tree).

Re-review is not automatic — after pushing the fix, remove and re-add the claude-review label to trigger a fresh pass.

Reviewed at HEAD: 36068a4

@PawelPeczek-Roboflow
PawelPeczek-Roboflow merged commit 6143d00 into main Jul 20, 2026
39 of 40 checks passed
@PawelPeczek-Roboflow
PawelPeczek-Roboflow deleted the codex/unreleased-version-handling branch July 20, 2026 18:37
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