Repository navigation
refactor: extract shared path-resolution helpers for console commands / コンソールコマンドのパス解決ヘルパーを共通化 - #583
Merged
zigzagdev merged 7 commits intoOct 4, 2026
Conversation
…h resolution normalizeLocalDiskPath/resolvePathToDir/resolvePathToFile were copy-pasted across multiple console commands. Extract them into a shared trait and have LoadsJsonRows delegate to it instead of duplicating the same logic.
Remove the duplicated normalizeLocalDiskPath/resolvePathToDir/ resolvePathToFile methods in favor of the shared trait.
Remove the duplicated normalizeLocalDiskPath method in favor of the shared trait.
…Name Remove the duplicated normalizeLocalDiskPath/resolvePath methods in favor of the shared trait, renaming the call site to resolvePathToFile.
…meFromJson Remove the duplicated resolvePath method in favor of the shared trait, renaming the call sites to resolvePathToFile.
…tiesFromSplitFile This class already uses LoadsJsonRows, which now provides resolvePath via ResolvesLocalDiskPaths. The local override was an exact duplicate of the trait's implementation.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## refactor/console-commands-cleanup #583 +/- ##
=======================================================================
+ Coverage 67.02% 67.85% +0.82%
+ Complexity 1707 1689 -18
=======================================================================
Files 149 151 +2
Lines 8929 8896 -33
=======================================================================
+ Hits 5985 6036 +51
+ Misses 2944 2860 -84
🚀 New features to boost your workflow:
|
Address codecov/patch failure on PR #583 (46.66% diff coverage) by directly testing normalizeLocalDiskPath/resolvePathToDir/resolvePathToFile for empty input, absolute paths, Windows drive paths, and relative paths resolved via the local disk.
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.
Motivation / 目的
normalizeLocalDiskPath/resolvePathToDir/resolvePathToFile/resolvePath were
copy-pasted across multiple UNESCO data pipeline console commands
(SplitWorldHeritageJson, DumpUnescoWorldHeritageJson,
DumpWorldHeritageSiteJapaneseName, ImportWorldHeritageJapaneseNameFromJson,
ImportSiteStatePartiesFromSplitFile, and the LoadsJsonRows trait). This made
the pipeline harder to maintain and was identified while reviewing the
Console/Commands directory for cleanup.
複数のUNESCOデータパイプライン用コンソールコマンドに同一のパス解決ロジックが
コピペされており、保守性を下げていました。Console/Commandsディレクトリの
整理の一環として対応します。
What I have done / 実施内容
ResolvesLocalDiskPathstrait consolidatingnormalizeLocalDiskPath/resolvePathToDir/resolvePathToFileLoadsJsonRowsto delegateresolvePath()to the new traitinstead of duplicating the same logic
SplitWorldHeritageJson,DumpUnescoWorldHeritageJson,DumpWorldHeritageSiteJapaneseName,ImportWorldHeritageJapaneseNameFromJson, and the redundant override inImportSiteStatePartiesFromSplitFileMakeManualSiteNamesJson::resolvePath()untouched — it behavesdifferently (uses
base_path(), doesn't strip theprivate/prefix),so unifying it would change behavior, not just dedupe
Test Results / テスト結果
php artisan test(full suite): 216 passed, no regressionsSplitWorldHeritageJsonIsTransboundaryTestandImportWorldHeritageSiteFromSplitFileIsTransboundaryTeststill pass,since they exercise the refactored path-resolution code paths