Skip to content

Conversation

@ryufuta
Copy link
Contributor

@ryufuta ryufuta commented Jan 2, 2026

Issue

概要

Prettierの実行コマンド中でglobパターンを引用符で囲んでいなかったため、シェル(bin/sh)がglob展開を実行しており、実行環境によっては意図した通りに展開されていなかった。
引用符で囲むようにしたことでglobの展開主体が実行環境によらずPrettier(内部的にnode-globを使用してglob展開を実行)に統一されるようにした。

補足

修正前はbin/shがBashバージョン4.0より前を指す環境ではapp/javascript/**/*.{js,jsx}app/javascript/*/*.{js,jsx}と解釈されていた。
そのため、app/javascript/直下のJavaScriptファイルなどがPrettierの対象外となっていて、検出されるべきエラーが検出されていなかった。

参考

変更確認方法

  1. chore/fix-inconsistent-glob-expansion-in-lintをローカルに取り込む
  2. app/javascript/action_completed_button.jsの適当な行末に;を追記する
    • app/javascript/**/*.{js,jsx}にマッチしてapp/javascript/*/*.{js,jsx}にはマッチしないファイルであれば何でも良い
    • 加える変更はPrettierで検出されるものであれば何でも良い
  3. ターミナル上でbin/yarn lintを実行すると2でlintエラーを混入させたファイルにエラーが検出されることを確認する

Summary by CodeRabbit

リリースノート

  • Chores
    • リント処理の構成を改善し、ゴブパターンのシェル展開を防ぐために処理スクリプトを更新しました。

注: この変更は内部開発プロセスの改善であり、ユーザー向け機能への直接的な影響はありません。

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai
Copy link

coderabbitai bot commented Jan 2, 2026

📝 Walkthrough

Walkthrough

package.jsonのlint:prettierスクリプトに、Prettierのグロブパターンの周囲にシングルクォートを追加し、シェル展開を防止するための変更を実施。

Changes

Cohort / File(s) 変更内容
パッケージスクリプト設定
package.json
lint:prettierスクリプトのグロブパターンにシングルクォートを追加。prettier app/javascript/**/*.{js,jsx} --checkからprettier 'app/javascript/**/*.{js,jsx}' --checkに変更し、シェルによるグロブ展開を回避

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related issues

  • ローカルの開発環境でPrettierが効かない #9469: このPRの変更(Prettierのグロブパターンの周囲にクォートを追加)により、ローカル環境とCI環境でのPrettierの動作の相違を引き起こしていたシェルグロブマッチングの問題に対処している。

Poem

🐰✨ グロブのまわりに クォートを巻きて
シェルの気まぐれ もう逃げられず
ローカルもCI も 仲よく動く
小さな変更 大きなちから 🌟

Pre-merge checks

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed プルリクエストのタイトルは、package.jsonのlint:prettierスクリプトでglobパターンを引用符で囲むという主要な変更を正確に説明しており、変更内容と完全に関連している。
Description check ✅ Passed プルリクエストの説明は、テンプレートに従い、Issue番号、変更の概要、補足説明、参考リンク、変更確認方法をすべて含んでおり、完全で詳細である。
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 60de05e and c095f18.

📒 Files selected for processing (1)
  • package.json
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: Miya096jp
Repo: fjordllc/bootcamp PR: 9102
File: app/controllers/mentor/buzzes_controller.rb:0-0
Timestamp: 2025-09-04T01:39:22.261Z
Learning: fjordllc/bootcamp プロジェクトでは、更新処理の成功時の通知メッセージについて、I18n を使用せずに日本語文字列を直接記述する方針で一貫性を保っている。
Learnt from: komagata
Repo: fjordllc/bootcamp PR: 9101
File: app/models/graduation_notifier.rb:23-28
Timestamp: 2025-09-12T21:16:47.639Z
Learning: Rails upgrade PRにおいて、configuration systemの変更やwebhook設定の改善提案も、アップグレードに直接関連しない場合は別PRで対応すべきである。PRのスコープ維持が重要。
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T08:23:30.927Z
Learning: Applies to app/javascript/**/*.{js,ts,jsx,tsx} : JavaScript/TypeScript code in `app/javascript/` should be linted with ESLint and formatted with Prettier via `yarn lint` scripts; use React 17 and Shakapacker/Webpack 5
Learnt from: ryufuta
Repo: fjordllc/bootcamp PR: 8992
File: app/models/correct_answer_notifier.rb:23-23
Timestamp: 2025-07-30T07:26:36.599Z
Learning: PRのスコープを維持することが重要であり、バグ修正PRにおいて、修正対象以外の機能(例:ルーティングテスト)の追加提案は適切ではない。そのような改善が必要な場合は別PRで対応すべきである。
📚 Learning: 2025-11-26T08:23:30.927Z
Learnt from: CR
Repo: fjordllc/bootcamp PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-26T08:23:30.927Z
Learning: Applies to app/javascript/**/*.{js,ts,jsx,tsx} : JavaScript/TypeScript code in `app/javascript/` should be linted with ESLint and formatted with Prettier via `yarn lint` scripts; use React 17 and Shakapacker/Webpack 5

Applied to files:

  • package.json
🔇 Additional comments (1)
package.json (1)

5-5: LGTM! グロブパターンの引用符追加により、シェル依存の問題を解決

この変更により、Prettierのグロブ展開がシェル環境に依存せず、Prettier内部で一貫して処理されるようになります。また、4行目のlint:eslintスクリプトと記法が統一され、保守性も向上しています。

PRの説明にあるとおり、この修正によりapp/javascript/直下のファイルが確実にチェック対象に含まれるようになります。


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@ryufuta ryufuta self-assigned this Jan 2, 2026
@ryufuta ryufuta marked this pull request as ready for review January 2, 2026 05:12
@github-actions github-actions bot requested a review from okuramasafumi January 2, 2026 05:12
@ryufuta ryufuta requested a review from tyrrell-IH January 5, 2026 05:33
@ryufuta
Copy link
Contributor Author

ryufuta commented Jan 5, 2026

@tyrrell-IH
お疲れ様です!
お手隙の際にレビューしていただけるとありがたいです🙏

@tyrrell-IH
Copy link
Contributor

@ryufuta
了解です〜、2・3日お待ちください!

Copy link
Contributor

@tyrrell-IH tyrrell-IH left a comment

Choose a reason for hiding this comment

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

エラーの再現、同エラーが今回のPRで解決できていること確認できました、approveします〜

シェルにglob展開をさせると環境によって結果に差異が出るんですね、勉強になりました🙏

@ryufuta
Copy link
Contributor Author

ryufuta commented Jan 7, 2026

@tyrrell-IH
レビューいただきありがとうございます🙏

@okuramasafumi
お手隙の際にレビューをお願いします🙏

Copy link
Contributor

@okuramasafumi okuramasafumi left a comment

Choose a reason for hiding this comment

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

リンク先の内容は勉強になりますね。シェルは歴史的経緯もあって色々難しい…
LGTM!

@ryufuta
Copy link
Contributor Author

ryufuta commented Jan 9, 2026

@okuramasafumi
レビューありがとうございます!
シェルの歴史をちょっと調べてみましたが、難しそうですね😅

@ryufuta
Copy link
Contributor Author

ryufuta commented Jan 9, 2026

@komagata
レビューが完了したのでマージをお願いします🙏

Copy link
Member

@komagata komagata left a comment

Choose a reason for hiding this comment

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

確認させていただきました。OKです〜🙆‍♂️

@komagata komagata merged commit e5fad6e into main Jan 13, 2026
13 checks passed
@komagata komagata deleted the chore/fix-inconsistent-glob-expansion-in-lint branch January 13, 2026 15:56
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.

5 participants