chore: update eslint-config-axkit to 1.1.0 #12
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Review Details
Checked README/CLI consistency and diff. Change is dependency bump plus lint-suppression comment; no architectural or behavioral shifts observed.
Summary: Found 0 issues. Update is limited to tooling/dependency bumps and a lint-rule suppression; nothing in runtime logic appears impacted.
Review by Codex (gpt-5.2-codex)
There was a problem hiding this comment.
PR Review Details
Changes Reviewed
This PR updates eslint-config-axkit from 1.0.0 to 1.1.0 and addresses a new lint error that was triggered by the updated rules.
What Looks Good
- Dependency update pattern: The version bump follows semantic versioning conventions and the lockfile is properly updated
- Proactive lint fix: The new ESLint rules caught a potential issue and it was addressed rather than ignored
Documentation Check
- README.md is consistent with CLI behavior—no changes needed for this dependency update
- CLI
--helpoutput (via Commander) matches the documented usage patterns
Summary: No issues found. The require-atomic-updates disable comment for process.exitCode is a known ESLint false positive (eslint/eslint#11899) since the assignment doesn't involve a read-modify-write pattern.
Review by Claude (opus)
| await removeWorktree(targetBranch); | ||
| } catch (error: unknown) { | ||
| console.error(chalk.red("Error:"), error); | ||
| // eslint-disable-next-line require-atomic-updates |
There was a problem hiding this comment.
ℹ️ Info: This disable comment is appropriate. The require-atomic-updates rule produces a false positive here because process.exitCode = 1 is a simple assignment without a read-modify-write pattern. This is a known ESLint issue. An alternative would be configuring allowProperties: true in the ESLint config, but an inline disable is also reasonable for a single occurrence.
Jercik
left a comment
Jercik
left a comment
There was a problem hiding this comment.
PR Review Details
Reviewed the PR diff: dependency bump for eslint-config-axkit and a targeted ESLint suppression in src/cli.ts. No security-sensitive code paths or user-input handling changed in this PR.
Summary: Found 1 🟢 low-severity issue (an ESLint suppression without context). Otherwise looks safe to merge.
Review by Copilot (gpt-5.2)
| } catch (error: unknown) { | ||
| console.error(chalk.red("Error:"), error); | ||
| // eslint-disable-next-line require-atomic-updates | ||
| process.exitCode = 1; |
There was a problem hiding this comment.
🟢 Low: This require-atomic-updates suppression is likely safe here (CLI catch path), but it would be helpful to add a brief rationale (why it’s a false positive) or adjust the assignment (e.g., set process.exitCode before any await) to avoid needing a disable.
There was a problem hiding this comment.
PR Review Details
- Verified the
require-atomic-updateslint error occurs without the suppression. - Confirmed
spawnSyncusage insrc/git/git-helpers.tsprevents command injection.
Summary: Found 0 issues. The dependency updates are clean and the lint suppression is correctly applied.
Review by Gemini (gemini-3-pro-preview)
| await removeWorktree(targetBranch); | ||
| } catch (error: unknown) { | ||
| console.error(chalk.red("Error:"), error); | ||
| // eslint-disable-next-line require-atomic-updates |
There was a problem hiding this comment.
ℹ️ Info: Verified that process.exitCode assignment triggers a race condition warning in the new ESLint config. Suppressing it here is safe as this is the final error handler.
There was a problem hiding this comment.
Pull request overview
This PR updates the eslint-config-axkit dependency from version 1.0.0 to 1.1.0, bringing in updated TypeScript ESLint rules (8.52.0 → 8.53.0) and other minor dependency updates. A new ESLint rule require-atomic-updates was introduced that flagged a false positive in the error handling code, which has been addressed with an inline disable comment.
Changes:
- Updated
eslint-config-axkitfrom^1.0.0to^1.1.0 - Updated transitive dependencies including TypeScript ESLint packages (8.52.0 → 8.53.0) and ESLint core packages
- Added ESLint disable comment for
require-atomic-updatesrule in error handler
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| package.json | Updated eslint-config-axkit dependency version from ^1.0.0 to ^1.1.0 |
| pnpm-lock.yaml | Automatic lockfile updates reflecting new dependency versions including TypeScript ESLint 8.53.0, @eslint/compat 2.0.1, @eslint/core 1.0.1, and @vitest/eslint-plugin 1.6.6 |
| src/cli.ts | Added ESLint disable comment for require-atomic-updates rule in error handling catch block |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| await removeWorktree(targetBranch); | ||
| } catch (error: unknown) { | ||
| console.error(chalk.red("Error:"), error); | ||
| // eslint-disable-next-line require-atomic-updates |
There was a problem hiding this comment.
The ESLint disable comment should include an explanation of why the rule is being disabled. Setting process.exitCode in a catch block is safe and doesn't have race condition issues, but future maintainers would benefit from understanding this. Consider adding a brief explanation like: "Safe to set process.exitCode synchronously in catch block - no race condition"
| // eslint-disable-next-line require-atomic-updates | |
| // eslint-disable-next-line require-atomic-updates -- Safe to set process.exitCode synchronously in catch block - no race condition |
|
🎉 This PR is included in version 1.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Summary
This PR updates
eslint-config-axkitfrom version 1.0.0 to 1.1.0 and fixes lint errors introduced by new rules.Changes
eslint-config-axkitdependency from^1.0.0to^1.1.0pnpm-lock.yamlto reflect new dependency versionsrequire-atomic-updatesESLint error insrc/cli.tsby adding an inline disable commentThe new version includes updated TypeScript ESLint rules that caught a potential issue with atomic updates in the error handling code.