Skip to content

Add release docs and skills#192

Merged
zgeorgiev-smartsheet merged 4 commits into
mainlinefrom
create-release-sdk-docs-and-skills
Jul 3, 2026
Merged

Add release docs and skills#192
zgeorgiev-smartsheet merged 4 commits into
mainlinefrom
create-release-sdk-docs-and-skills

Conversation

@zgeorgiev-smartsheet

Copy link
Copy Markdown
Contributor

Adds a formal release procedure for the SDK, documenting the step-by-step process (version bump, changelog update, PR, GitHub Release, automated npm publish). Also adds an AI agent skill so that agents can execute the release workflow.

@coveralls

coveralls commented Jul 1, 2026

Copy link
Copy Markdown

Coverage Report for CI Build 28657076201

Warning

No base build found for commit 880278d on mainline.
Coverage changes can't be calculated without a base build.
If a base build is processing, this comment will update automatically when it completes.

Coverage: 95.386%

Details

  • Patch coverage: No coverable lines changed in this PR.

Uncovered Changes

No uncovered changes found.

Coverage Regressions

Requires a base build to compare against. How to fix this →


Coverage Stats

Coverage Status
Relevant Lines: 1271
Covered Lines: 1226
Line Coverage: 96.46%
Relevant Branches: 246
Covered Branches: 221
Branch Coverage: 89.84%
Branches in Coverage %: Yes
Coverage Strength: 1735.06 hits per line

💛 - Coveralls

Comment thread .claude/skills/release/SKILL.md Outdated
Comment thread .claude/skills/release/SKILL.md Outdated
@ggoranov-smar

Copy link
Copy Markdown
Contributor

Code Review

Reviewed the three changed files at head cd75d1e, cross-checking every concrete claim against the real repo config: package.json, both .github/workflows/ files, CHANGELOG.md, .husky/, .npmignore, and git tag/commit history.

Bottom line: accurate, high-quality docs PR. The claims easiest to get wrong — the npm publish trigger, OIDC/no-token, the CHANGELOG insertion pattern, tag format — are all correct and verified against the actual c8242ec "Prepare for release v5.1.0" commit and the real workflow YAML. Issues below are mostly consistency/completeness, not factual errors.

Strengths

  • build-publish.yaml claims are exactly right. Both docs state publish is triggered by a published GitHub Release running npm cinpm run buildnpm publish via OIDC with no stored token. The real workflow matches precisely (release: types: [published]; id-token: write; ci/build/publish steps). The "must be published, not draft" warning is a correct and valuable callout.
  • CHANGELOG procedure verified against real history. RELEASE.md's diff block and SKILL.md's before/after both match the actual 5.1.0 prep diff in c8242ec (header inserted between the permanent ## [X.X.X] - Unreleased line and ### Added).
  • Tag/version conventions match git history. vX.X.X matches all existing tags. PR title Prepare for release vX.X.X matches prior release commits. Base branch mainline is correct.
  • npm run build before publish is correctly required — there is no prepublishOnly/prepare build hook and dist/ is not committed, so the explicit build step is genuinely necessary.
  • Sound edge-case coverage rarely in release docs: rollback via patch + npm deprecate, the 72-hour un-publish window, and a "Common Mistakes" table.
  • Valid skill frontmatter — proper name + "Use when…" description trigger; directory name matches.

Issues

Critical (Must Fix)

None. No factually wrong step and no destructive command that would break a release.

Important (Should Fix)

  1. Procedure duplicated across three files — guaranteed to rot. RELEASE.md, SKILL.md, and the new AGENTS.md section each restate the version rules, the "Files Changed in Every Release" table, the CI steps, and tag/PR conventions. They agree today, but the next process change must be hand-synced in three places — the exact way "docs contradict tooling" happens. Fix: make RELEASE.md the single source of truth (it already declares itself "Authoritative reference"); reduce SKILL.md and the AGENTS.md section to a short "when to use + read RELEASE.md" pointer plus only the agent-specific bits.

  2. SKILL.md step 7 issues git commit + git push + gh pr create with no confirmation gate. An agent following this will push a branch and open a PR autonomously. Fix: add an explicit "confirm with the user before committing/pushing/opening the PR" gate, and note the destructive nature of the later Publish step. (The actual npm publish is safely gated behind the manual GitHub UI — good.)

  3. SKILL.md "Before" example contradicts the "empty placeholder" description. Step 3 shows a "Before" state with content (### Added / - ...) under ## [X.X.X] - Unreleased, while RELEASE.md and the same file's Rules call the Unreleased block a permanently "empty placeholder." In reality feature PRs accumulate entries under Unreleased, then release-prep inserts the version header above them — so "empty" is misleading. Fix: change "empty placeholder" to "placeholder header" so it's consistent with the Before example.

  4. Node version prerequisite drift vs. what CI publishes on. RELEASE.md says "Node.js 20+" and volta.node pins 20.19.0, but build-publish.yaml publishes on Node 24 (lint runs only on 24; coverage matrix is 20/22/24). Fix: note that CI builds/publishes on Node 24 while supporting 20/22/24.

  5. Docs omit the prettier format gate in CI. RELEASE.md describes CI as "lint + coverage matrix," but test-build.yaml runs both npm run lint (eslint) and npm run format (prettier --check). A PR that lints clean but isn't prettier-formatted will fail CI. Fix: mention npm run format as part of the CI gate (and optionally that .husky/pre-commit runs lint-staged locally).

Minor (Nice to Have)

  • SKILL.md step 9 vs RELEASE.md step 5 verification driftnpm view smartsheet@X.X.X vs npm view smartsheet versions --json | tail -5. Folds into the single-source-of-truth recommendation.
  • Skill directory naming style differs from siblings — existing skills use imperative verb-noun (implement-api-endpoint); this one is a gerund (releasing-smartsheet-javascript-sdk). Cosmetic.

Recommendations

  • Collapse to one source of truth: full procedure in RELEASE.md; SKILL.md and the AGENTS.md Release Agent section link to it and carry only their delta.
  • Add a short "If CI fails / if publish fails" subsection to RELEASE.md (currently only success path + rollback are covered): e.g., publish failed after the release was published → the release/tag already exist, so re-running requires re-triggering the workflow or cutting a patch.
  • Add an explicit human-confirmation gate in SKILL.md before the commit/push/PR step and before instructing publication.

Assessment

Ready to merge? With fixes.

Reasoning: Every load-bearing factual claim (publish trigger, OIDC no-token, CHANGELOG insertion pattern, tag/PR conventions, build-before-publish) is verified correct against the real workflows and git history, so there are no Critical blockers. The Important items — triplicated procedure that will drift, a missing confirmation gate before the agent commits/pushes, the "empty placeholder" wording contradiction, the Node-version drift, and the omitted prettier CI gate — should be addressed before merge.

🤖 Generated with Claude Code

@zgeorgiev-smartsheet
zgeorgiev-smartsheet merged commit 48e6999 into mainline Jul 3, 2026
5 checks passed
@zgeorgiev-smartsheet
zgeorgiev-smartsheet deleted the create-release-sdk-docs-and-skills branch July 3, 2026 12:40
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