Ci/judge commits not pr title - #535
Merged
Merged
Conversation
Fix(zh-Hant): CI 不再檢查 PR 標題與描述，只檢查分支上每一則 commit Fix(en-US): CI no longer checks the PR title and body, only the commits on the branch The title defaults to the branch name — `Fix/version week` — and the body can be empty, so the gate was failing pull requests over two fields nobody wrote. The commits are the thing somebody actually wrote, and they were already being judged. What this gives up has to be said plainly: a squash merge commits the title and body, not those commits, so squashing now writes an unread message to main. The only backstop is main's own push event running the gate again, which is detection after the fact. Merge by rebase and what lands is what was judged — and each commit keeps its own changelog entries and its own snapshot marker instead of collapsing them into one.
Fix(zh-Hant): 修正 Linux 上會誤判「SDK 未安裝」而拒絕執行 Fix(en-US): fix the toolchain guard wrongly refusing to run on Linux `mise exec` runs a binary, not a shell line, and `command` is a shell builtin. macOS happens to ship /usr/bin/command as a real executable and Linux does not, so `mise exec -- command -v flutter` answered correctly on a laptop and answered nothing on every Linux runner — which the guard then reported as "the SDK is not installed", on a runner where mise had installed it. That is exactly the works-here-fails-there this function exists to stop, so it now asks mise directly, and the message says what it probed rather than asserting a cause it did not establish.
|
🔍 OpenCodeReview found 5 issue(s) in this PR.
|
Fix(zh-Hant): Android 與 iOS 的建置改用釘選的 Flutter 版本，不再跟著當日 stable 走 Fix(en-US): the Android and iOS builds now use the pinned Flutter, not whatever stable is today Both jobs used subosito/flutter-action with `channel: stable`, which resolves to whatever stable is on the day. They were the last two places shipping artifacts built against an SDK nobody chose, and nothing in a build log says which SDK produced it — the whole reason the pin exists. They now install mise and call the same tool/ scripts a developer calls. The iOS SPM cache was also pointing at DerivedData, which flutter never uses for packages: it always passes -clonedSourcePackagesDirPath = build/ios/SourcePackages. The cache stored nothing, so every run paid the cold resolve the cache was added to avoid — 19-32 s, measured. `tool/dev/build.sh ios` drops its hardcoded --release, which would have fought the --debug the iOS smoke-test build passes. `flutter build ios` is release by default, so nothing else changes.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
Comment on lines
93
to
104
| @@ -102,7 +104,7 @@ jobs: | |||
| EOF | |||
There was a problem hiding this comment.
秘密應透過 env: 區塊傳遞給需要的步驟，而不是直接在 run: 中使用 ${{ secrets.X }}。這可以避免秘密意外洩露到日誌中，並符合安全最佳實踐。
Suggestion:
Suggested change
| - name: Decode keystore | |
| run: | | |
| echo "$KEYSTORE_BASE64" | base64 --decode > android/app/my-release-key.jks | |
| env: | |
| KEYSTORE_BASE64: ${{ secrets.KEYSTORE_BASE64 }} | |
| - name: Create key.properties | |
| run: | | |
| cat > android/key.properties << EOF | |
| storePassword=$KEYSTORE_PASSWORD | |
| keyPassword=$KEY_PASSWORD | |
| keyAlias=$KEY_ALIAS | |
| storeFile=my-release-key.jks | |
| EOF | |
| env: | |
| KEYSTORE_PASSWORD: ${{ secrets.KEYSTORE_PASSWORD }} | |
| KEY_PASSWORD: ${{ secrets.KEY_PASSWORD }} | |
| KEY_ALIAS: ${{ secrets.KEY_ALIAS }} |
Comment on lines
+44
to
+45
| - name: Install toolchain (mise) | ||
| uses: jdx/mise-action@v4 |
There was a problem hiding this comment.
第三方 Action jdx/mise-action@v4 使用了版本標籤 (tag) 而非完整的提交雜湊值 (commit SHA)。雖然這比使用 channel: stable 更具決定性，但為了防止標籤被篡改（供應鏈攻擊），建議將其鎖定在特定的 commit SHA。
Suggestion:
Suggested change
| - name: Install toolchain (mise) | |
| uses: jdx/mise-action@v4 | |
| - name: Install toolchain (mise) | |
| uses: jdx/mise-action@<commit_sha> |
Fix(zh-Hant): 修正更新日誌的署名會在超過 30 則 commit 或被限流時無聲消失 Fix(en-US): release-note attribution no longer disappears silently past 30 commits or under a rate limit Three faults, all of which produced a release note that looked finished and credited nobody. Actions does not put GITHUB_TOKEN in the environment; a step has to ask, and this one never did. So `authors_of` resolved every pull request unauthenticated, at 60 requests an hour shared across everything on that runner's IP. That limit is not theoretical: the API answered 403 while this was being written, with 0 of 60 remaining. The commits endpoint pages at 30 by default and nothing said so, so a pull request with more commits than that lost every author past the thirtieth. The last one had 63. It asks for 100 now, which is the maximum; the endpoint itself stops at 250 whatever is asked for, and past that the co-author trailers are the only record left. And every failure was silent — `2>/dev/null || true` over a body that parses whether it is a commit list or an error object. It now reads the status and says which call failed and why, because a note with no attribution and no complaint is indistinguishable from a release nobody contributed to.
Fix(zh-Hant): 修正換行的更新日誌條目在發布時被從中間截斷 Fix(en-US): a wrapped changelog entry is no longer published cut off mid-sentence The extractor's regex is whole-line and commit.md's own example wraps an entry across two lines, so the indented remainder matched nothing and was dropped in silence. Three English sentences shipped in 26w34b ending mid-clause — "a large event is shown at full extent on the monitor and the" — with "replay map" left behind in the commit nobody reads. Nothing failed. The note built, the release published, and the only symptom was a sentence that stops. So the body is folded before extraction: an indented line continues the one above it, which is what the format already documented and the tool did not implement. A blank line still ends an entry, so the prose an author writes underneath is not swallowed into it.
Comment on lines
+44
to
+45
| - name: Install toolchain (mise) | ||
| uses: jdx/mise-action@v4 |
There was a problem hiding this comment.
原本使用 subosito/flutter-action@v2 並開啟了 cache: true，這會自動快取 Flutter/Dart 的 pub cache。現在改用 jdx/mise-action@v4 後，若沒有額外透過 actions/cache 設定快取 ~/.pub-cache（或對應路徑），每次執行 tool/dev/deps.sh 都必須重新下載所有套件，這將會顯著增加建置時間。
Comment on lines
+22
to
+27
| final start = script.indexOf("git log -1 --format=%b \"\$sha\" | awk '"); | ||
| expect(start, isNot(-1), reason: 'the fold is no longer where it was'); | ||
| final awk = script.substring( | ||
| script.indexOf("'", start) + 1, | ||
| script.indexOf("')", start), | ||
| ); |
There was a problem hiding this comment.
在 test/tool/release_notes_wrap_test.dart 的 fold 函數中，對 tool/release/notes.sh 的實作細節存在高度耦合。該函數透過 indexOf 搜尋特定的指令字串來提取 awk 腳本，這意味著一旦 notes.sh 的程式碼格式（如空格、引號或指令結構）發生任何非邏輯性的變動，測試將會因為找不到對應位置而失敗，導致測試非常脆弱（Brittle）。建議考慮將 awk 邏輯提取到獨立檔案，或者在測試中更穩健地定位該邏輯。
Comment on lines
+42
to
+44
| bool isEntry(String line) => RegExp( | ||
| r'^(New|Optimization|Fix)\(([A-Za-z]{2,3}(-[A-Za-z0-9]+)*)\):\s*(.+)$', | ||
| ).hasMatch(line); |
There was a problem hiding this comment.
測試檔案中的 isEntry 函數使用了硬編碼的正規表示法（Regex）來驗證結果。若 tool/release/notes.sh 修改了其用來識別條目的正規表示法邏輯（例如增加新的類型或修改格式），但測試中的 isEntry 未同步更新，將會導致測試結果無法真實反映腳本的功能變化，進而造成誤判。建議 isEntry 的 Regex 應與 notes.sh 中的 LINE_RE 保持一致。
lowrt
approved these changes
Aug 18, 2026
Only rebase merging is allowed now, so a commit on main is the commit somebody wrote and its author is the author. The path that resolved `(#N)` back to a pull request's own commits existed because a squash sets the author to whoever pressed merge; with squashing off it answers a question nobody asks any more. The cost is taken deliberately, not overlooked: the 282 commits already on main that arrived by squash keep their merge-time author. Trailers still rescue the ones that have them — `41a3c1e8` still credits @PiscesXD — but only 7 of the most recent 40 do, and `c5fdbd31` in the next note will credit the merger for 53 entries somebody else wrote. The comments that still described squashing went with it: the gate's `--message` mode is a draft validator now and says so, and its failure no longer tells anyone to go edit a pull request title.
Comment on lines
+44
to
+45
| - name: Install toolchain (mise) | ||
| uses: jdx/mise-action@v4 |
There was a problem hiding this comment.
jdx/mise-action@v4 是一個第三方 Action，目前使用的是 tag (@v4) 而非完整的 Commit SHA。根據安全最佳實踐，建議將第三方 Action 鎖定在特定的 Commit SHA，以防止該 tag 被篡改或指向惡意代碼，從而降低供應鏈攻擊的風險。
Comment on lines
+22
to
+27
| final start = script.indexOf("git log -1 --format=%b \"\$sha\" | awk '"); | ||
| expect(start, isNot(-1), reason: 'the fold is no longer where it was'); | ||
| final awk = script.substring( | ||
| script.indexOf("'", start) + 1, | ||
| script.indexOf("')", start), | ||
| ); |
There was a problem hiding this comment.
fold 函式的實作方式過於脆弱（Brittle）。它使用 indexOf 並匹配一個非常具體的字串來定位並提取 awk 腳本。一旦 tool/release/notes.sh 中的命令格式（例如空格、引號或參數順序）發生任何微小變化，該測試就會因為找不到匹配字串而失效，增加了維護成本。建議改用更穩健的方式（例如使用正規表示式或更具彈性的字串搜尋）來定位 awk 腳本塊。
Comment on lines
+85
to
+88
| final body = | ||
| Process.runSync('git', ['log', '-1', '--format=%b', '41a3c1e8']).stdout | ||
| as String; | ||
| if (body.trim().isEmpty) return; // shallow clone |
There was a problem hiding this comment.
針對特定 Commit (41a3c1e8) 的回歸測試，在進行淺層複製（shallow clone）的 CI 環境中會因找不到該 Commit 而直接跳過 (if (body.trim().isEmpty) return;)。這可能導致關鍵的真實案例驗證未能執行。雖然這在某些情況下是合理的，但如果這是該測試的核心價值所在，建議考慮將該 Commit 的內容作為測試資料的一部分直接寫入測試腳本中，以確保 CI 環境下的測試一致性與有效性。
Comment on lines
+154
to
+155
| status="${body##*$'\n'}" | ||
| body="${body%$'\n'*}" |
There was a problem hiding this comment.
在 api_json 函數中，使用 shell 的參數擴展 ${body%$'\n'*} 來分離 HTTP 狀態碼與 JSON Body 的方式存在潛在邊界條件風險。如果 GitHub API 回傳的 JSON Body 本身結尾包含多個換行符，或者 Body 的結構在特定情況下導致最後一行與狀態碼的分割不符合預期，可能會導致 Body 被錯誤地截斷，進而使後續的 Python JSON 解析失敗。建議使用更嚴謹的方式來分離狀態碼與 Body，例如 curl -w 輸出的最後一行一定是狀態碼，可以用 tail -n 1 來獲取。
Comment on lines
+257
to
+261
| body="$(git log -1 --format=%b "$sha" | awk ' | ||
| /^[[:space:]]+[^[:space:]]/ && held { sub(/^[[:space:]]+/, " "); printf "%s", $0; next } | ||
| { if (held) printf "\n"; printf "%s", $0; held = 1 } | ||
| END { if (held) printf "\n" } | ||
| ')" |
There was a problem hiding this comment.
在處理 Commit Body 的 awk 邏輯中，對縮排內容的展開處理過於通用，可能會破壞 Commit Message 的原始格式。該邏輯會將所有以空格開頭的行（如程式碼塊、Markdown 清單或縮排的說明文字）強制與上一行合併，這在生成 Release Notes 時可能會導致原本有意義的格式混亂或程式碼塊被壓扁成一行。
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
2 participants
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.
這個 PR 做了什麼
相關 issue
怎麼驗
檢查清單
tool/check/commits.sh origin/main..HEAD通過—— commit 訊息就是更新日誌，格式見 commit.md
mise exec -- flutter analyze與mise exec -- flutter test通過AppLocalizations，沒有寫死AppSpacing/AppRadius/AppMotion，深色模式看過，文字對比度可接受