Extend release-notes cut to valkey-search, valkey-json, valkey-bloom - #80
Open
sarthakaggarwal97 wants to merge 1 commit into
Open
Extend release-notes cut to valkey-search, valkey-json, valkey-bloom#80sarthakaggarwal97 wants to merge 1 commit into
sarthakaggarwal97 wants to merge 1 commit into
Conversation
Per-repo release conventions live in a ProjectProfile registry (scripts/release_notes/projects.py), keyed by repo name so personal forks resolve the upstream conventions: - valkey: src/version.h VALKEY_* macros (unchanged behavior) - valkey-search: src/version.h kModuleVersion + MODULE_RELEASE_STAGE - valkey-json: CMakeLists.txt project(... VERSION M.m.p) - valkey-bloom: Cargo.toml [package] version The changelog renderer, prior-section splitter, rc-sequence reader, commit titles, and AI prompts are parameterized by the profile. With the previous fixed "Valkey M.m.p" splitter, a module heading such as "Valkey Search 1.2.1" never matched and a cut would have silently dropped the repo's entire prior changelog. Repos whose version file records no stage compare M.m.p only in the version-file gate; tag validation stays authoritative, and a changelog-heading gate refuses a duplicate GA or an rc dispatched in the merge-to-tag window. Legacy hand-written contributor blocks are absorbed into the cumulative footer on the first cut (fail-closed parsing); name-only co-author identities merge with their handle. profile is a required argument through the internal plumbing so a dropped pass-through fails loudly. The dispatch gains a repo choice, the App token is minted for the selected repo after an allowlist gate, and concurrency is per-repo. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
Collaborator
Author
|
@KarthikSubbarao @Aksha1812 would you guys want to review this? Tested with a dry run against the real valkey-search 1.2 branch: run |
Collaborator
Author
|
@BChan-0 thoughts on this? |
📝 WalkthroughWalkthroughThe release-notes system now supports Valkey, Valkey Search, Valkey JSON, and Valkey Bloom. Repository profiles control version files, release stages, prompts, categories, rendering, changelog handling, and publishing workflow behavior. ChangesRepository-aware workflow
Profile-driven release pipeline
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/release_notes/release_cut.py (1)
647-675: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider running the duplicate-stage gate before the AI run.
_refuse_already_cut_stageruns only insidecut, afterregenerate_unreleasedcompletes. For a stage-less version file a duplicate GA or a post-GA rc therefore consumes a full triage and generation run before the cut aborts.main._validate_release_targetalready clones the release line and reads the version file, so it can read the changelog and apply the same gate first.This does not change correctness; it only fails fast.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@scripts/release_notes/release_cut.py` around lines 647 - 675, Apply _refuse_already_cut_stage during main._validate_release_target, after cloning the release line and reading its version/changelog, so duplicate-stage and post-GA checks happen before regenerate_unreleased or any AI work. Preserve the existing gate in cut as needed for correctness, using the same version, stage, profile, and changelog inputs.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/release-notes-cut.yml:
- Line 142: Update the HAS_APP_CREDS expression in the release workflow to be
true only when both VALKEYRIE_BOT_APP_ID and VALKEYRIE_BOT_PRIVATE_KEY are
non-empty, so the token action is skipped and the dry-run PAT fallback remains
available when either credential is missing.
---
Nitpick comments:
In `@scripts/release_notes/release_cut.py`:
- Around line 647-675: Apply _refuse_already_cut_stage during
main._validate_release_target, after cloning the release line and reading its
version/changelog, so duplicate-stage and post-GA checks happen before
regenerate_unreleased or any AI work. Preserve the existing gate in cut as
needed for correctness, using the same version, stage, profile, and changelog
inputs.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 03a3a4e4-f41f-4b50-a05b-38125dad054b
📒 Files selected for processing (19)
.github/workflows/release-notes-cut-advanced.yml.github/workflows/release-notes-cut.ymlREADME.mddocs/architecture.mdscripts/release_notes/generate.pyscripts/release_notes/main.pyscripts/release_notes/pipeline.pyscripts/release_notes/projects.pyscripts/release_notes/release_cut.pyscripts/release_notes/release_format.pyscripts/release_notes/render.pyscripts/release_notes/triage.pytests/test_release_notes_main.pytests/test_release_notes_pipeline.pytests/test_release_notes_projects.pytests/test_release_notes_release_cut.pytests/test_release_notes_release_format.pytests/test_release_notes_rerun.pytests/test_release_notes_workflow.py
| CLAUDE_CODE_USE_BEDROCK: "1" | ||
| # secrets.* is not readable in step `if:` expressions; surface presence | ||
| # here so the token steps can be skipped on a fork without App creds. | ||
| HAS_APP_CREDS: ${{ secrets.VALKEYRIE_BOT_APP_ID != '' }} |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Require both App credential secrets.
Line 142 treats an App ID alone as usable credentials. If VALKEYRIE_BOT_PRIVATE_KEY is empty, a token action runs and fails before the permitted dry-run PAT fallback can execute. Require both secrets.
Proposed fix
- HAS_APP_CREDS: ${{ secrets.VALKEYRIE_BOT_APP_ID != '' }}
+ HAS_APP_CREDS: ${{ secrets.VALKEYRIE_BOT_APP_ID != '' && secrets.VALKEYRIE_BOT_PRIVATE_KEY != '' }}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| HAS_APP_CREDS: ${{ secrets.VALKEYRIE_BOT_APP_ID != '' }} | |
| HAS_APP_CREDS: ${{ secrets.VALKEYRIE_BOT_APP_ID != '' && secrets.VALKEYRIE_BOT_PRIVATE_KEY != '' }} |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/release-notes-cut.yml at line 142, Update the
HAS_APP_CREDS expression in the release workflow to be true only when both
VALKEYRIE_BOT_APP_ID and VALKEYRIE_BOT_PRIVATE_KEY are non-empty, so the token
action is skipped and the dry-run PAT fallback remains available when either
credential is missing.
Contributor
This looks like a good implementation; Might require testing against other cases (looks like you tested a cut for a patch release, but I'd try logic for major and minor releases too, especially major since the regex might not parse as cleanly with the different repositories' different formatting on the unstable files). I think legacy contributor dedup might also be flaky with different formatting in -bloom and -json(?) but that can either be solved manually or passing new format allowances. I think more dry runs may be helpful to dig up other cases. Otherwise, it should at least be nondestructive to run since it's same discovery logic and is independent of the core valkey notes anyways. |
sarthakaggarwal97
added a commit
to sarthakaggarwal97/valkey-ci-agent
that referenced
this pull request
Aug 8, 2026
release-start (authorize, derive, open tracker, chain the cut), release-reconcile (hourly recompute + guarded progress actions, with a valkey token deliberately lacking contents:write and a separate downstream write token excluding valkey), release-adopt (record an authorized owner's acknowledgement of branch movement), and release-publish (validate renders the approval evidence; the publish job runs behind the protected release environment, which the code verifies is actually protected - required reviewers, no self-review, no admin bypass - before any path runs). The notes cut becomes fork-safe: its target repo is owner-scoped instead of hardcoded upstream, App-token steps tolerate missing secrets, and VALKEY_GITHUB_TOKEN is accepted as the fork fallback. Every PAT fallback is structurally impossible when the owner is valkey-io. Rebase point with PR valkey-io#80 (repo input on the cut) is marked in release-start. Signed-off-by: Sarthak Aggarwal <sarthagg@amazon.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extend the release-notes workflow beyond Valkey core to support automated release cuts for:
valkey-searchvalkey-jsonvalkey-bloomRepository-specific behavior now lives in a
ProjectProfileregistry instead of being hard-coded for Valkey. Each repository can define its own version source, release-stage handling, changelog identity, note categories, and generation prompts.What changed
valkey:src/version.husingVALKEY_*macrosvalkey-search:src/version.husingkModuleVersionandMODULE_RELEASE_STAGEvalkey-json:CMakeLists.txtproject versionvalkey-bloom:Cargo.tomlpackage version