Skip to content

リファクタリング Phase 1: イディオム・スタイル最新化#86

Merged
limit7412 merged 2 commits into
masterfrom
refactor/phase-1-idioms
Jun 15, 2026
Merged

リファクタリング Phase 1: イディオム・スタイル最新化#86
limit7412 merged 2 commits into
masterfrom
refactor/phase-1-idioms

Conversation

@limit7412

@limit7412 limit7412 commented Jun 15, 2026

Copy link
Copy Markdown
Owner

Part of #84(3分割 PR の 1/3)

最新の Crystal(1.20系)らしいイディオム・スタイルへ更新。挙動は不変

  • shard.yml: crystal 要件を >= 1.20.0
  • lambda.cr: puts `echo '...'` のサブシェルログ(インジェクション/クォート破壊リスク)を stdlib Log に置換。handler のブロックパラメータを明示
  • github/repository.cr: 数値比較を HTTP::Status 述語(server_error? / unauthorized? / client_error?)へ。typo (retrun/faild) 修正
  • github/models.cr: propertygetter、冗長な emit_null: false 削除、[...].includes?→定数配列+in?comment_urlpresence ||
  • error/usecase.cr: err.backtracebacktrace?.try で安全に
  • notify/usecase.cr: size != 0empty? 否定
  • slack/models.cr: propertygetter

検証: crystal tool format --check 差分なし / crystal build 成功。

後続: Phase 2 (#87) → Phase 3 (#88) がこの上にスタックされます。

🤖 Generated with Claude Code

- shard.yml: bump crystal requirement to >= 1.20.0
- lambda.cr: replace echo-subshell logging with stdlib Log, explicit block param
- github/repository.cr: use HTTP::Status predicate methods, fix typos
- github/models.cr: property -> getter, constant arrays + in?, presence
- usecase/error: idiomatic || and backtrace? safe navigation
- slack/models.cr: property -> getter, formatter

refs #84

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request refactors the codebase to use modern Crystal idioms, such as changing properties to getters, utilizing semantic HTTP status checks, and simplifying conditional logic. It also replaces shell execution in print_log with the standard Log library. The review feedback suggests further performance optimizations: using Tuple instead of Array for static constants (MENTION_REASONS and UPDATE_TYPES) to avoid heap allocations, and refactoring print_log to use index-based string slicing instead of each_char.each_slice to prevent intermediate array allocations.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread src/github/models.cr Outdated
Comment thread src/github/models.cr Outdated
Comment thread src/runtime/lambda.cr
limit7412 added a commit that referenced this pull request Jun 15, 2026
Address review feedback on PR #86:
- MENTION_REASONS / UPDATE_TYPES: Array -> Tuple to avoid heap allocation
- print_log: replace each_char.each_slice with index-based String slicing
  to avoid intermediate Array(Char)

refs #84

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Address review feedback on PR #86:
- MENTION_REASONS / UPDATE_TYPES: Array -> Tuple to avoid heap allocation
- print_log: replace each_char.each_slice with index-based String slicing
  to avoid intermediate Array(Char)

refs #84

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@limit7412 limit7412 force-pushed the refactor/phase-1-idioms branch from de50b63 to f704124 Compare June 15, 2026 06:29
@limit7412 limit7412 merged commit c6f7835 into master Jun 15, 2026
1 check passed
@limit7412 limit7412 deleted the refactor/phase-1-idioms branch June 15, 2026 06:42
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.

1 participant