Skip to content

fix(guardrails): address PR #154 review — remove unused import, deduplicate merge regex#157

Merged
terisuke merged 1 commit intodevfrom
fix/pr154-review-followup
Apr 10, 2026
Merged

fix(guardrails): address PR #154 review — remove unused import, deduplicate merge regex#157
terisuke merged 1 commit intodevfrom
fix/pr154-review-followup

Conversation

@terisuke
Copy link
Copy Markdown

Summary

  • Remove unused list import from guardrail-context.ts
  • Compute isMerge/isPrMerge booleans once at function entry in bashBeforeGit and reuse throughout, eliminating 12 duplicated regex evaluations
  • Same deduplication applied to afterBashGit handler (afterIsMerge, afterIsPrMerge, afterIsPrCreate, afterIsPush)

Addresses inline review comments on PR #154.

Test plan

  • No logic change — regex results are cached in variables instead of re-evaluated
  • Build passes (guardrails package only)

🤖 Generated with Claude Code

…licate merge regex

- Remove unused `list` import from guardrail-context.ts
- Compute isMerge/isPrMerge booleans once in bashBeforeGit and reuse
- Same pattern applied to afterBashGit handler (afterIsMerge, afterIsPrMerge, etc.)

Addresses review comments on PR #154.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings April 10, 2026 08:33
@github-actions
Copy link
Copy Markdown

New PR opened -- automated review will run on the next push.

To trigger a manual review, comment /review on this PR.

@github-actions
Copy link
Copy Markdown

This PR doesn't fully meet our contributing guidelines and PR template.

What needs to be fixed:

  • PR description is missing required template sections. Please use the PR template.

Please edit this PR description to address the above within 2 hours, or it will be automatically closed.

If you believe this was flagged incorrectly, please let a maintainer know.

@github-actions
Copy link
Copy Markdown

Thanks for your contribution!

This PR doesn't have a linked issue. All PRs must reference an existing issue.

Please:

  1. Open an issue describing the bug/feature (if one doesn't exist)
  2. Add Fixes #<number> or Closes #<number> to this PR description

See CONTRIBUTING.md for details.

@github-actions
Copy link
Copy Markdown

The following comment was made by an LLM, it may be inaccurate:

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR follows up on review feedback from PR #154 by removing an unused import and reducing repeated command-regex evaluations in the guardrails git handlers, without changing behavior.

Changes:

  • Removed an unused list import from guardrail-context.ts.
  • Cached isMerge / isPrMerge once in bashBeforeGit() and reused them throughout.
  • Applied the same caching approach in bashAfterGit() (afterIsMerge, afterIsPrMerge, afterIsPrCreate, afterIsPush).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/guardrails/profile/plugins/guardrail-git.ts Deduplicates repeated merge/push/PR-create regex tests by caching booleans per invocation.
packages/guardrails/profile/plugins/guardrail-context.ts Removes an unused import to satisfy lint/typecheck and keep the context module tidy.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@terisuke terisuke merged commit e2ba7df into dev Apr 10, 2026
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants