docs(skills): eccube-asset を esbuild ベースで再追加し pre-push ゲート・PHPUnit の 500 切り分けを追記 - #7022
Open
ttokoro20240902 wants to merge 4 commits into
Open
docs(skills): eccube-asset を esbuild ベースで再追加し pre-push ゲート・PHPUnit の 500 切り分けを追記#7022ttokoro20240902 wants to merge 4 commits into
ttokoro20240902 wants to merge 4 commits into
Conversation
先行 PR のマージ待ちになる知見をまとめた追随 PR（Draft）。 - eccube-asset: #7008 から外したものを #7013 の esbuild.config.mjs に合わせて 書き直して再追加。SCSS エントリの自動検出（_ 始まりでない .scss が対象）、 CSS の <style> 注入、ace の allowlist 配置、source map の相対パス正規化を反映。 Bootstrap dist CSS への lint 誤検知も「よくある間違い」に追加。 - eccube-contributing: .husky/pre-push が push のたびに rector 全体走査と phpstan analyze src/ を実行することを追記。変更外ファイルで落ちたときは vendor と composer.lock のずれ（依存のバージョンずれ／クラスマップ陳腐化）を 疑う切り分けを追加。 - eccube-phpunit: createFormData がキーを送らないフィールドで非 nullable setter に null が渡り 500 になる件。DataMapper は現在値と同じならスキップするため CI では再現せずローカルだけ落ちる。 - AGENTS.md: Skill 索引表に eccube-asset の行を戻す。
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughEC-CUBE 4.4 のアセットビルド、push フック、PHPUnit テストに関する開発 Skill 文書を更新しました。 Changes開発ガイダンス
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
6 tasks
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4.4 #7022 +/- ##
==========================================
+ Coverage 77.17% 77.20% +0.03%
==========================================
Files 564 564
Lines 28082 28082
==========================================
+ Hits 21671 21680 +9
+ Misses 6411 6402 -9
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
先行 PR (#6937 / #7013) のマージを取り込む。 .claude/skills/eccube-phpunit/SKILL.md の「よくある間違い」で衝突したため、 両側の項目を残したうえで AGENTS.md の歯止め（1 Skill 10 項・1 項 120 字程度、 超過分は追記ではなく統合）に沿って統合した。 - 4.4 側 3 項（未宣言プロパティ / 非 nullable プロパティ / HTML パート無しメール）と 本ブランチ側 1 項（ローカルだけ 500 になるテスト）はいずれも情報を保持している。 - 未宣言プロパティと非 nullable プロパティは原因が同じ「プロパティ宣言」なので 1 項に統合。 - HTTP クライアント / URL / Entity の自前生成、ID・ステータス値の直書き、 回帰テストのゲート未確認と PHP Warning も同種のため統合した。 - 結果として 13 項・最大 411 字 → 8 項・最大 143 字。 AGENTS.md では「既存の超過は次にその節へ手を入れるときに統合・短縮する」と 定めており、衝突解消でこの節に触るため、あわせて実施した。
#7013 のマージで esbuild が入ったので、eccube-asset の記述を esbuild.config.mjs とリポジトリの実体で 1 項目ずつ照合した。 実体と合っていなかった点、および実装にあって書かれていなかった点を直す。 eccube-asset - 「対象」の SCSS ソースに install を挙げていたが、 html/template/install に scss ディレクトリは存在せず JS エントリだけ。 SCSS は default / admin の 2 つに訂正する。 - 「生成物」を html/template/*/assets/css/*.css と広く書いていたが、 実際に scss から生成されるのは style.* / app.* / bootstrap.* の 3 系統のみ。 install/assets/css/dashboard.css と admin/assets/css/tempusdominus-bootstrap-4*.css は対応する scss が無い 手管理ファイルで、再ビルドしても更新されない。両者を書き分ける。 - 上記を踏まえ「css/ 配下すべてを生成物と決めつける」を「よくある間違い」に追加。 eccube-contributing - .husky/pre-push は dev コンテナ XML が無いとき先に bin/console cache:clear --env=dev を実行する（無いと rector が全ファイル read error で落ちるため）。初回 push が長い理由が分からないと 異常と誤認するので追記する。 eccube-phpunit - 依存ライブラリの例外メッセージを全文アサートしない、を追加。 twig/twig 3.28.0 の「Report the column number in syntax errors」 (twigphp/Twig#4834) で `at line N.` が `at line N column M.` に変わり、 composer.lock の更新だけで PHPUnit の全マトリクスが落ちた実例がある。 composer.json の制約は ^3.21 で 3.28 も許容するため、版差で変わらない 部分だけを含有判定する。 なお TwigLintValidatorTest 自体の修正は #7024 に含まれるため本 PR では触らない。
ttokoro20240902
marked this pull request as ready for review
August 6, 2026 00:41
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.claude/skills/eccube-asset/SKILL.md:
- Around line 20-23: SKILL.md の css/ 編集方針を、生成された CSS
のみに適用されるよう更新してください。dashboard.css と tempusdominus-bootstrap-4*.css
は手管理ファイルとして編集禁止の対象外であることを明記し、生成物のみが次回ビルドで上書きされる説明に修正してください。
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3138f2d3-429a-4dfe-bf18-2a70db3da70d
📒 Files selected for processing (4)
.claude/skills/eccube-asset/SKILL.md.claude/skills/eccube-contributing/SKILL.md.claude/skills/eccube-phpunit/SKILL.mdAGENTS.md
CodeRabbit の指摘に対応する。 本 PR で「css/ に混在する非生成物」（install/assets/css/dashboard.css と admin/assets/css/tempusdominus-bootstrap-4*.css は対応する scss を持たない 手管理ファイル）を追記した結果、既存の「css/ 配下を直接編集しない。次のビルドで 上書きされて消える」と矛盾していた。手管理ファイルは scss が無いため直接編集する しかなく、ビルドで上書きもされない。 編集禁止の対象を「生成された css/」に限定し、手管理ファイルが例外である旨を 明記する。css/ に言及する他 11 箇所は「生成物」と限定済みで矛盾しないことを 確認した。
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
概要
Skill の知見追加のうち、先行 PR のマージを待つ必要があるものをまとめた追随 PR です。Draft で置いています。
内容は 2 系統あります。
1. eccube-asset を esbuild ベースで再追加
eccube-assetは当初 #7008 に含めていましたが、#7013（esbuild への移行）でgulpfile.js/webpack.config.jsが削除されるため、そのままでは内容が古くなります。#7008 に残すと esbuild とは無関係な他 9 Skill の知見も一緒に足止めされるので、eccube-assetだけを #7008 から外し、本 PR で #7013 のesbuild.config.mjsに合わせて書き直したものを再追加します。gulp 版から変わった点を反映しています。
npm run build（=node esbuild.config.mjs）の 1 コマンドで SCSS と JS バンドルの両方が通る。監視はnpm starthtml/template配下の_始まりでない.scss）。部分ファイルに_を付け忘れると単独の CSS として出力されてしまう<style>として注入される（style-loader 相当）ため、テンプレート側の読み込み方は変わらないaceFilesにも追加が必要で、足し忘れはビルドでは検出されず実行時に壊れるsourcesは相対パスへ正規化される。生成物をコミットする運用でビルドマシンのパスが焼き込まれるのを避けるためあわせて、生成物（
css/min.css/map/html/bundle）が git 管理下であるという前提と、Bootstrap の dist CSS に対する lint 誤検知（:not(:-moz-placeholder)は Bootstrap が意図的に分割出力しているもので、そもそもビルド生成物なので手編集不可）も「よくある間違い」に入れています。2. pre-push ゲートと PHPUnit の 500 切り分け
.husky/pre-pushの存在が Skill に書かれておらず、CI ゲートの表も 4 つしか挙げていませんでした。push のたびに rector（全体走査）とphpstan analyze src/が走ることを追記し、変更していないファイルで落ちたときはvendor/とcomposer.lockのずれを疑うという切り分けも入れています（依存のバージョンずれと、ファイル移動後のクラスマップ陳腐化の 2 パターン）。判断の拠り所は「CI が同じゲートで緑か」です。createFormData()がキー自体を送っていないフィールドがあると、DataMapper が非 nullable な setter にnullを渡して 500 になります。DataMapper は送信値が現在値と同じならセットをスキップするため、その列が NULL の CI では再現せず、値が入っているローカルだけで落ちるという紛らわしい形になります。自分の変更に帰属させる前にcatchExceptions(false)で実際の例外を見る、という切り分けを追記しました。ready にする条件（すべて充足）
このブランチは base を
4.4にしています（スタック PR にしていません）。先行 PR は両方マージされたため、4.4 を取り込んで解決しました。eccube-phpunit/eccube-contributingは improvement: deprecation ゲート有効化＋未使用の非推奨 public API 削除 (#6933) #6937 も触っているため、追記が衝突しないことを確認するesbuild.config.mjsとeccube-assetの記述を突き合わせる取り込み時の対応
eccube-phpunitの「よくある間違い」が衝突しました。 本 PR 側の 1 項（ローカルだけ 500 になるテスト）と#6937 側の 3 項（未宣言プロパティ / 非 nullable プロパティ / HTML パート無しメール）が同じ位置への追加同士でした。
両側の情報を残したうえで、
AGENTS.mdの歯止め（1 Skill 10 項・1 項 120 字程度、超過分は追記ではなく統合）に沿って統合しています。AGENTS.mdが「既存の超過は次にその節へ手を入れるときに統合・短縮する」と定めており、衝突解消でこの節に触るためあわせて実施しました。ID・ステータス値の直書き / 回帰テストのゲート未確認＋PHP Warning
esbuild.config.mjsとの突き合わせで、記述が実体と合っていない点が見つかったので直しました。eccube-assetの「対象」で SCSS ソースにinstallを挙げていましたが、html/template/installにscssは無く JS エントリだけです（SCSS はdefault/adminの 2 つ）html/template/*/assets/css/*.cssと広く書いていましたが、scss から生成されるのはstyle.*/app.*/bootstrap.*のみで、install/assets/css/dashboard.cssとadmin/assets/css/tempusdominus-bootstrap-4*.cssは対応する scss を持たない手管理ファイルです（再ビルドしても更新されない）。書き分けたうえで「よくある間違い」にも 1 項追加しましたeccube-contributing:.husky/pre-pushは dev コンテナ XML が無いとき先にbin/console cache:clear --env=devを実行します。初回 push が長い理由が分からないと異常と誤認するため追記しましたeccube-phpunit: 依存ライブラリの例外メッセージを全文アサートしない、を追加しました。twig/twig 3.28.0 の「Report the column number in syntax errors」（Track the source offset of each token and expose it in syntax errors twigphp/Twig#4834）で
at line N.がat line N column M.に変わり、composer.lockの更新だけで PHPUnit の全マトリクスが落ちた実例があります（chore(deps): bump the php-production group across 1 directory with 7 updates #7024 / chore(deps): bump the symfony group across 1 directory with 47 updates #7026）。composer.jsonの制約は^3.21で 3.28 も許容するため、版差で変わらない部分だけを含有判定します。なお
TwigLintValidatorTest自体の修正は chore(deps): bump the php-production group across 1 directory with 7 updates #7024 に含まれるため本 PR では触っていませんテスト
Markdown のみの変更でコードの変更はありません（
git diff origin/4.4...HEADは Skill 3 ファイル＋AGENTS.mdの索引 1 行）。記述内容は 4.4 の実コードで裏取りしています。
esbuild.config.mjs（npm run build/npm start、SCSS エントリの自動検出、style-inject、aceFilesの allowlist、source map の相対パス正規化）.husky/pre-push（rector 全体走査＋phpstan analyze src/、ECCUBE_HOOK_RUNNER、HUSKY=0、cold 状態のcache:clear）git ls-filesによる生成物の追跡状態とパーミッション（.cssは 100755 /.mapは 100644）、installにscssが無いことローカルで CI と同じゲートを実行済み（vendor は
composer installで lock に同期）。vendor/bin/phpstan analyse src→[OK] No errorsvendor/bin/rector process --dry-run→[OK] Rector is done!.husky/pre-push（push 時に実走）→pre-push: all checks passed関連
eccube-assetをこちらへ移した）eccube-phpunit/eccube-contributingを触る先行 PR）Summary by CodeRabbit