mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-08-13 10:44:30 +00:00
450dd6d1d6
* fix(prompt): replace the fullwidth colon in the file_read tool description tools.json advertised the example output as "File：path/to/example.go" with a fullwidth colon (U+FF1A), while file_read.go actually emits "File: %s". The description is sent to the model on every review, so the example did not match the output it was describing. Also switches action.yml's OCR_LANGUAGE example from 中文 to Chinese, for the same reason as #861: the value is fed to the LLM and Chinese is what the rest of the project uses. * chore(ci): fail CI when CJK characters appear in source files Comments, identifiers and strings in this repository are meant to be English, but nothing enforced it — #861 had to clean up leftovers by hand, and the same drift keeps arriving through generated code and contributions written internally. scripts/verify-cjk.go walks the index plus untracked files and reports Han ideographs, kana, CJK punctuation and fullwidth forms. Written in Go rather than shell so it does not depend on the container's grep having PCRE, and so `unicode.Is` decides what counts as CJK instead of a byte range that would flag the em dashes used throughout the comments. `//go:build ignore` keeps it out of ./..., so it does not affect go vet, go build or the coverage threshold. Untracked files are included (--others --exclude-standard) so a new file is checked before it lands: while writing this, the script's own comment used Chinese punctuation as an example and went unreported until it was staged. Two escape hatches, preferring the narrow one: an `allow-cjk: <reason>` marker comment on a single line, or a prefix in allowedPrefixes for a whole tree. 23 existing lines get markers (UTF-8 encoding fixtures, multibyte truncation fixtures, language-switcher labels, the fullwidth bar used as a terminal cursor). pages/src/i18n/ is allowlisted as translated UI copy; extensions/vscode/ is allowlisted TEMPORARILY — its comments, test names and zh-cn NLS bundle are still Chinese and need a follow-up pass. Wired into CI next to the license and action-pin checks, plus `make cjk-check` and `make check` for local runs. * chore(ci): generalise the CJK check to all non-English text Addresses the review feedback, and widens the rule that the feedback exposed. Review feedback: - exemptMarker requires its colon, so a bare "allow-cjk" can no longer exempt a line without giving a reason. - The script is named for CJK but missed Hangul. - git ls-files gains -z, so paths that are not plain ASCII arrive unquoted, and its stderr is reported rather than a bare exit status. - main discarded run()'s error entirely and only called os.Exit(1), which is what made the lost stderr invisible in the first place. - The CI step and AGENTS.md say "unapproved", since escape hatches exist. The check was skewed by writing system rather than by language. In one array the 'zh' and 'ja' labels each needed a marker while the adjacent 'ru' label passed untouched, and nine lines of Russian sat in the tree unflagged: two language-switcher labels and the heading-ID fixtures. Contributors writing Chinese had to justify every line; contributors writing Russian had nothing to justify. The rule is now "a letter outside ASCII", since written English needs no letter beyond the ASCII 26 -- Cyrillic and Han as much as the diacritics that spell German or Vietnamese. Scripts are not enumerated, so one nobody has contributed in yet is covered when it arrives. Common and Inherited pass, so letterlike symbols (U+2139, U+2113) are not mistaken for prose, and combining accents are caught, so the decomposed spelling of an accented letter cannot slip through. Symbols and emoji stay out of scope by construction: they are not letters. Renamed to scripts/verify-english-only.go and make english-check, and the marker to allow-non-english:. Text spelled entirely in ASCII still takes a dictionary to identify and stays a matter for review. * docs(agents): restate the English-only rule as rule, homes, hatches The rule was one dense bullet that led with the detection mechanism and mentioned the exemptions only in passing, which is the wrong order for the reader: an agent needs to know where a translation may go before it needs to know which Unicode scripts are flagged. Split into three. The homes are now spelled out from what the tree actually holds, rather than left as "<locale> docs or an i18n table": README and CONTRIBUTING in zh-CN, ja-JP, ko-KR and ru-RU; the doc pages under pages/src/content/docs/ in en, zh, ja and ru; the UI copy tables in pages/src/i18n/. Also why the two are exempt for different reasons -- Markdown by extension, the i18n tables by prefix because they are .ts -- since that decides where a new translation can safely go. Drops the enumerated list of what "make check" runs. It duplicated the Makefile, went stale the moment a check was added (this PR had to edit it), and told an agent nothing it would not read in the output anyway. What is worth saying is that the target writes to the tree. * fix(ci): detect U+FE10–FE6F CJK punctuation in english-only check The vertical forms (U+FE10–FE19), CJK compatibility forms (U+FE30–FE4F) and small form variants (U+FE50–FE6F) were not caught, even though their fullwidth counterparts (U+FF00–FFEF) already were. A small question mark (U+FE56 ﹖) or vertical comma (U+FE10 ︐) left in source reads as correct English punctuation and is invisible in review — the same class of typo the fullwidth range already defends against. Skip U+FE20–FE2F (Combining Half Marks) which are used in Latin text.
115 lines
3.4 KiB
Makefile
115 lines
3.4 KiB
Makefile
.PHONY: build test clean run help fmt vet check coverage \
|
|
build-all dist sha256sum version-info \
|
|
build-linux-amd64 build-linux-arm64 build-darwin-amd64 build-darwin-arm64 \
|
|
build-windows-amd64 build-windows-arm64 \
|
|
license-check license-add english-check
|
|
|
|
BINARY_NAME := opencodereview
|
|
GO := go
|
|
DIST_DIR := ./dist
|
|
|
|
# Version info — use git tag if available, fallback to short commit hash
|
|
GIT_TAG := $(shell git describe --tags --abbrev=0 2>/dev/null || echo "")
|
|
GIT_COMMIT := $(shell git rev-parse --short HEAD)
|
|
BUILD_DATE := $(shell date -u +"%Y-%m-%dT%H:%M:%SZ")
|
|
|
|
VERSION ?= $(if $(GIT_TAG),$(GIT_TAG),v0.0.0-$(GIT_COMMIT))
|
|
|
|
LD_FLAGS := \
|
|
-X main.Version=$(VERSION) \
|
|
-X main.GitCommit=$(GIT_COMMIT) \
|
|
-X main.BuildDate=$(BUILD_DATE)
|
|
|
|
RELEASE_LD_FLAGS := -s -w $(LD_FLAGS)
|
|
|
|
define BUILD_PLATFORM
|
|
GOOS=$(1) GOARCH=$(2) CGO_ENABLED=0 $(GO) build -ldflags "$(RELEASE_LD_FLAGS)" \
|
|
-o $(DIST_DIR)/$(BINARY_NAME)-$(1)-$(2)$(3) \
|
|
./cmd/opencodereview
|
|
endef
|
|
|
|
# ── Development targets ──────────────────────────────────────────────────────
|
|
build:
|
|
$(GO) build -ldflags "$(LD_FLAGS)" -o $(DIST_DIR)/$(BINARY_NAME) ./cmd/opencodereview
|
|
|
|
PACKAGES := $(shell $(GO) list ./... | grep -v /extensions/)
|
|
|
|
test:
|
|
LC_ALL=C $(GO) test -v -race -count=1 $(PACKAGES)
|
|
|
|
COVERAGE_THRESHOLD := 90
|
|
|
|
coverage:
|
|
LC_ALL=C $(GO) test -count=1 -coverprofile=coverage.out $(PACKAGES)
|
|
$(GO) tool cover -func=coverage.out | grep total:
|
|
@COVERAGE=$$($(GO) tool cover -func=coverage.out | grep total: | awk '{print $$3}' | sed 's/%//'); \
|
|
if awk "BEGIN {exit !($$COVERAGE < $(COVERAGE_THRESHOLD))}"; then \
|
|
echo "FAIL: Coverage $${COVERAGE}% is below $(COVERAGE_THRESHOLD)% threshold"; \
|
|
exit 1; \
|
|
fi; \
|
|
echo "PASS: Coverage $${COVERAGE}% meets $(COVERAGE_THRESHOLD)% threshold"
|
|
|
|
clean:
|
|
rm -rf $(DIST_DIR) coverage.out
|
|
|
|
run: build
|
|
$(DIST_DIR)/$(BINARY_NAME) --staged
|
|
|
|
help: build
|
|
$(DIST_DIR)/$(BINARY_NAME) -h
|
|
|
|
fmt:
|
|
gofmt -s -w .
|
|
|
|
vet:
|
|
LC_ALL=C $(GO) vet $(PACKAGES)
|
|
|
|
check: license-check english-check
|
|
$(GO) mod tidy
|
|
gofmt -s -w .
|
|
LC_ALL=C $(GO) vet $(PACKAGES)
|
|
@echo "check passed"
|
|
|
|
license-check:
|
|
@bash scripts/verify-license.sh
|
|
|
|
english-check:
|
|
@$(GO) run scripts/verify-english-only.go
|
|
|
|
license-add:
|
|
@bash scripts/add-license.sh
|
|
|
|
# ── Cross-platform targets ───────────────────────────────────────────────────
|
|
build-linux-amd64:
|
|
$(call BUILD_PLATFORM,linux,amd64)
|
|
|
|
build-linux-arm64:
|
|
$(call BUILD_PLATFORM,linux,arm64)
|
|
|
|
build-darwin-amd64:
|
|
$(call BUILD_PLATFORM,darwin,amd64)
|
|
|
|
build-darwin-arm64:
|
|
$(call BUILD_PLATFORM,darwin,arm64)
|
|
|
|
build-windows-amd64:
|
|
$(call BUILD_PLATFORM,windows,amd64,.exe)
|
|
|
|
build-windows-arm64:
|
|
$(call BUILD_PLATFORM,windows,arm64,.exe)
|
|
|
|
build-all: build-linux-amd64 build-linux-arm64 build-darwin-amd64 build-darwin-arm64 build-windows-amd64 build-windows-arm64
|
|
|
|
# Generate SHA256 checksums for all release binaries
|
|
sha256sum: build-all
|
|
cd $(DIST_DIR) && shasum -a 256 $(BINARY_NAME)-* | sort > sha256sum.txt
|
|
|
|
# Full release: clean → build all platforms → checksums
|
|
dist: clean build-all sha256sum
|
|
@echo $(VERSION) > $(DIST_DIR)/VERSION
|
|
|
|
version-info:
|
|
@echo "Version: $(VERSION)"
|
|
@echo "GitCommit: $(GIT_COMMIT)"
|
|
@echo "BuildDate: $(BUILD_DATE)"
|
|
@echo "LD_FLAGS: $(LD_FLAGS)"
|