-
Notifications
You must be signed in to change notification settings - Fork 0
Release/0.1.1 #5
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
WalkthroughAdds release automation: new Changes
Sequence Diagram(s)mermaid 🚥 Pre-merge checks | ✅ 1 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@scripts/release.js`:
- Around line 83-89: The script currently stages and commits all uncommitted
changes using runCapture('git status --porcelain') and run(...) which is risky;
change the behavior so that when status is non-empty (the variable status from
runCapture) the script aborts with a clear error instructing the user to
review/clean their working directory (rather than calling run('git add .') and
run(`git commit -m "chore: prepare for v${newVersion} release"`)). Update the
logic around status in the release flow to exit non-zero with a helpful message
referencing newVersion, and remove or disable the automatic run('git add
.')/run(...commit...) calls so commits are only done manually after the user
verifies changes.
🧹 Nitpick comments (3)
scripts/release.js (3)
25-38: Version parsing doesn't handle pre-release versions.The
split('.').map(Number)approach will produceNaNfor versions containing pre-release identifiers (e.g.,1.0.0-beta.1). Whilst this may be acceptable for your current workflow, consider adding validation.🔧 Optional: Add basic validation
function calculateNewVersion(current, type) { const [major, minor, patch] = current.split('.').map(Number) + + if ([major, minor, patch].some(n => isNaN(n) || n < 0)) { + throw new Error(`Invalid version format: ${current}`) + } switch (type) {
103-112: Hardcoded repository URL reduces portability.The PR URL is hardcoded to
jackmisner/utm-toolkit. If this script is ever used in a fork or the repository is renamed, the link will be incorrect.🔧 Dynamically derive the repository URL from the git remote
+function getRepoUrl(remote) { + const url = runCapture(`git remote get-url ${remote}`) + // Convert SSH to HTTPS format and remove .git suffix + return url + .replace(/^git@github\.com:/, 'https://github.com/') + .replace(/\.git$/, '') +} + function main() { // ... existing code ... const remote = getRemoteName() + const repoUrl = getRepoUrl(remote) // ... later in the success message ...Then update the message:
-1. Create a PR: https://github.com/jackmisner/utm-toolkit/compare/main...${branchName} +1. Create a PR: ${repoUrl}/compare/main...${branchName}
91-101: Consider rollback on failure.If
npm versionorgit pushfails, the script leaves behind a partially created release branch. This isn't critical but could cause confusion for subsequent release attempts.You could wrap these operations in a try-catch and delete the local branch on failure:
try { run(`git checkout -b ${branchName}`) run(`npm version ${type}`) run(`git push -u ${remote} ${branchName} --follow-tags`) } catch (error) { console.error('❌ Release failed, cleaning up...') run('git checkout main') run(`git branch -D ${branchName}`) process.exit(1) }
… you to manually review and commit first.
Summary by CodeRabbit
✏️ Tip: You can customise this high-level summary in your review settings.