refactor(release-cleanup): 將清理邏輯由 bash 改寫為 Node.js #5

Closed
jiantw83 wants to merge 43 commits from refactor/nodejs-rewrite-20260626-103443 into develop
Showing only changes of commit 87db9030d6 - Show all commits
+6 -2
View File
1
@@ -17,8 +17,12 @@ export function selectReleasesToDelete(releases, keepCount) {
const time = Date.parse(release?.created_at)
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Rogue
問題:selectReleasesToDelete 針對每一筆 release 重複執行 Date.parse,浪費 CPU 週期。
建議:應在排序前先執行一次 map 轉換,預先計算時間戳記。

**嚴重等級**:🟡 警告 **審查員**:Rogue **問題**:selectReleasesToDelete 針對每一筆 release 重複執行 Date.parse,浪費 CPU 週期。 **建議**:應在排序前先執行一次 map 轉換,預先計算時間戳記。
return Number.isNaN(time) ? 0 : time
}
const sorted = [...releases].sort((a, b) => createdTime(b) - createdTime(a))
return sorted.slice(keepCount)
// 先一次性計算每筆的時間戳,避免在排序比較中重複呼叫 Date.parse。
Ghost marked this conversation as resolved
Review

嚴重等級🟡 警告
審查員:Leo
問題:在 selectReleasesToDelete 函式中,對於 Date.parse 無法解析的日期直接視為 0,這在資料清理邏輯中是一個隱晦的行為,未來維護者可能不清楚為什麼無效日期會優先被刪除。
建議:應明確記錄無效日期的處理方式(例如記錄 warning),或在 Date.parse 失敗時,應考慮給予一個明確的邏輯(例如拋出錯誤或放到特定排序位置),以減少不可預期的副作用。

**嚴重等級**:🟡 警告 **審查員**:Leo **問題**:在 `selectReleasesToDelete` 函式中,對於 `Date.parse` 無法解析的日期直接視為 `0`,這在資料清理邏輯中是一個隱晦的行為,未來維護者可能不清楚為什麼無效日期會優先被刪除。 **建議**:應明確記錄無效日期的處理方式(例如記錄 warning),或在 `Date.parse` 失敗時,應考慮給予一個明確的邏輯(例如拋出錯誤或放到特定排序位置),以減少不可預期的副作用。
return releases
Ghost marked this conversation as resolved Outdated
Outdated
Review

嚴重等級🔴 嚴重
審查員:Maya
問題:cleanupReleases 是執行刪除舊成品的核心函式,但目前缺乏針對 API 呼叫失敗(如 GET 失敗、DELETE 失敗)的測試,無法驗證錯誤處理邏輯是否如預期運作。
建議:使用測試框架搭配 mock 伺服器回應,補齊 cleanupReleases 的測試案例,特別是驗證當 deleteResource 回傳非 204 狀態碼時,系統是否正確記錄錯誤並繼續或中斷。

**嚴重等級**:🔴 嚴重 **審查員**:Maya **問題**:cleanupReleases 是執行刪除舊成品的核心函式,但目前缺乏針對 API 呼叫失敗(如 GET 失敗、DELETE 失敗)的測試,無法驗證錯誤處理邏輯是否如預期運作。 **建議**:使用測試框架搭配 mock 伺服器回應,補齊 cleanupReleases 的測試案例,特別是驗證當 deleteResource 回傳非 204 狀態碼時,系統是否正確記錄錯誤並繼續或中斷。
.map((release) => ({ release, time: createdTime(release) }))
.sort((a, b) => b.time - a.time)
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Rogue
問題:為了排序而進行了多次 map 操作,先將陣列 map 為物件陣列,排序後又 map 回原始物件陣列。這在大數據集下會造成多次 O(n) 的陣列分配與垃圾回收壓力,浪費 CPU 與記憶體週期。
建議:排序時應嘗試減少陣列中間狀態的產生,例如考慮使用原地排序(若不介意原陣列變更)或簡化 mapping 的邏輯。

**嚴重等級**:🔵 建議 **審查員**:Rogue **問題**:為了排序而進行了多次 map 操作,先將陣列 map 為物件陣列,排序後又 map 回原始物件陣列。這在大數據集下會造成多次 O(n) 的陣列分配與垃圾回收壓力,浪費 CPU 與記憶體週期。 **建議**:排序時應嘗試減少陣列中間狀態的產生,例如考慮使用原地排序(若不介意原陣列變更)或簡化 mapping 的邏輯。
Review

嚴重等級🟡 警告
審查員:Mage
問題:在 selectReleasesToDelete 的排序邏輯中,若多個成品具有完全相同的 created_at 時間戳,目前的排序行為依賴於 JavaScript 引擎對 sort() 的實作(在某些情況下可能不穩定),導致保留與刪除的成品選擇具有不確定性。
建議:建議在排序邏輯中加入次要的排序鍵值(如 idtag_name)作為比較依據(例如:若時間相同,則比較 ID 大小),以確保排序結果在時間相同時仍具有決定性。

**嚴重等級**:🟡 警告 **審查員**:Mage **問題**:在 `selectReleasesToDelete` 的排序邏輯中,若多個成品具有完全相同的 `created_at` 時間戳,目前的排序行為依賴於 JavaScript 引擎對 `sort()` 的實作(在某些情況下可能不穩定),導致保留與刪除的成品選擇具有不確定性。 **建議**:建議在排序邏輯中加入次要的排序鍵值(如 `id` 或 `tag_name`)作為比較依據(例如:若時間相同,則比較 ID 大小),以確保排序結果在時間相同時仍具有決定性。
.map((entry) => entry.release)
.slice(keepCount)
}
/**
Ghost marked this conversation as resolved
Review

嚴重等級🔵 建議
審查員:Bard
問題:在 cleanupReleases 函式中,參數 configReturnType<import('./config.js').loadConfig>,這種型別定義方式雖然準確,但過於冗長且與實作細節耦合過深,影響程式碼的可讀性與簡潔度。
建議:建議在 app/config.js 定義並匯出型別註解(JSDoc @typedef),然後在此處直接使用該別名,提升整體程式碼的可讀性。

**嚴重等級**:🔵 建議 **審查員**:Bard **問題**:在 `cleanupReleases` 函式中,參數 `config` 是 `ReturnType<import('./config.js').loadConfig>`,這種型別定義方式雖然準確,但過於冗長且與實作細節耦合過深,影響程式碼的可讀性與簡潔度。 **建議**:建議在 `app/config.js` 定義並匯出型別註解(JSDoc @typedef),然後在此處直接使用該別名,提升整體程式碼的可讀性。
Review

嚴重等級🔵 建議
審查員:Bard
問題:config 參數型別定義方式雖然準確,但過於冗長且與實作細節耦合過深,影響程式碼的可讀性與簡潔度。
建議:建議在 app/config.js 定義並匯出型別註解(JSDoc @typedef),然後在各處直接使用該別名。

**嚴重等級**:🔵 建議 **審查員**:Bard **問題**:config 參數型別定義方式雖然準確,但過於冗長且與實作細節耦合過深,影響程式碼的可讀性與簡潔度。 **建議**:建議在 `app/config.js` 定義並匯出型別註解(JSDoc @typedef),然後在各處直接使用該別名。
19