mirror of
https://github.com/alibaba/open-code-review.git
synced 2026-10-02 01:15:23 +08:00
fix(config): exclude dependency and build-output directories by default (#1499)
* fix(config): exclude dependency and build-output directories by default
The default list covered tests, snapshots and generated code but not the
directories a package manager writes. **/oh_modules/** was there and
**/node_modules/** was not.
.gitignore does not stand in for this. Once a directory has been committed
git diff stops consulting .gitignore for it, so reviewing that commit or any
range spanning it pulls in every file -- this repository committed
extensions/frontend/node_modules and one review picked up 389 files. The
extension allowlist does not either: the contents are .js, .ts, .json and
.css, all supported, so they reach the path gate unfiltered.
**/build/** and **/bin/** are deliberately absent. Projects keep hand-written
sources in both, including this repository's own bin/ocr.js, and a false
exclusion is silent.
One behaviour change worth naming: **/vendor/** supersedes the old
extension-scoped **/vendor/**/*.{jsonnet,libsonnet}, so vendored Go and PHP
sources are now excluded where they previously were not. Two test cases
asserted the old behaviour and now assert the new one.
Fixes #1494
* docs(review): describe default_path as more than a test-file gate
review-rules.md and architecture.md both said default_path matches built-in
test-file patterns, and that noisy directories are filtered earlier at the diff
level. After the new patterns the first is incomplete and the second is only
true at the repository root: providerDirIgnoreDirs matches by path prefix, so
vendor/pkg/x.go never reaches the per-file filter while api/vendor/pkg/x.go
does and is excluded by default_path instead.
That distinction is user-visible, because an include rule can override a
default_path exclusion and cannot override the provider blocklist. Both
documents now say which one reports which, and the pattern list covers the
dependency and build-output group alongside the test group.
Five locales each.
* feat: Apply batched suggestions from code review
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
---------
Co-authored-by: kite <lizhengfeng.lzf@alibaba-inc.com>
Co-authored-by: Kite <254839944+lizhengfeng101@users.noreply.github.com>
This commit is contained in:
@@ -250,9 +250,13 @@ func TestWhyExcluded_UserIncludePattern(t *testing.T) {
|
||||
{
|
||||
name: "non-included file with valid extension still reviewed (additive semantics)",
|
||||
diff: model.Diff{
|
||||
NewPath: "vendor/baz.go",
|
||||
// Was vendor/baz.go, chosen only as a path outside the include
|
||||
// patterns that nothing else excluded. #1494 added **/vendor/**
|
||||
// to the default list, so it needs a path that is still only
|
||||
// non-included and not excluded on any other ground.
|
||||
NewPath: "cmd/baz.go",
|
||||
},
|
||||
// .go is a supported extension and vendor/baz.go does not hit
|
||||
// .go is a supported extension and cmd/baz.go does not hit
|
||||
// IsExcludedPath, so it falls through to ExcludeNone.
|
||||
expected: ExcludeNone,
|
||||
},
|
||||
|
||||
@@ -303,14 +303,16 @@ func TestIsExcludedPath(t *testing.T) {
|
||||
{"capnp in filename only", "src/capnp_helpers.go", false},
|
||||
|
||||
// Jsonnet vendored dependencies (written by `jb install`, wiped by `rm -rf vendor`).
|
||||
// The pattern is extension-scoped: IsExcludedPath applies every pattern to every
|
||||
// path, so a bare **/vendor/** would also drop vendored Go and PHP sources.
|
||||
// This used to be extension-scoped to keep vendored Go and PHP sources reviewable.
|
||||
// #1494 reversed that on purpose: vendor/ is a dependency directory whatever the
|
||||
// language inside it, and those two cases below now expect exclusion. The narrow
|
||||
// **/vendor/**/*.{jsonnet,libsonnet} pattern is gone, superseded by **/vendor/**.
|
||||
{"jsonnet vendor root", "vendor/github.com/grafana/jsonnet-libs/ksonnet-util/kausal.libsonnet", true},
|
||||
{"jsonnet vendor nested dir", "jsonnet/vendor/foo/main.jsonnet", true},
|
||||
{"jsonnet non-vendor lib", "lib/config.libsonnet", false},
|
||||
{"jsonnet non-vendor env", "environments/prod/main.jsonnet", false},
|
||||
{"go under vendor still reviewed", "vendor/github.com/pkg/errors/errors.go", false},
|
||||
{"php under vendor still reviewed", "vendor/monolog/monolog/src/Logger.php", false},
|
||||
{"go under vendor", "vendor/github.com/pkg/errors/errors.go", true},
|
||||
{"php under vendor", "vendor/monolog/monolog/src/Logger.php", true},
|
||||
// Zig test files
|
||||
{"zig test directory", "test/parser.zig", true},
|
||||
{"zig nested test directory", "src/test/unit/parser.zig", true},
|
||||
@@ -391,6 +393,76 @@ func TestIsExcludedPath(t *testing.T) {
|
||||
{"hdl tb without underscore not excluded", "rtl/tbench.v", false},
|
||||
{"hdl tb substring mid-name not excluded", "rtl/outbound.v", false},
|
||||
|
||||
// Dependency directories and build output (#1494). Files inside these
|
||||
// carry ordinary .js/.ts/.json/.css extensions, so the extension gate
|
||||
// passes them through and only a path pattern stops them. The real case
|
||||
// that prompted this: extensions/frontend/node_modules was committed
|
||||
// once, and from then on git diff stopped consulting .gitignore for it,
|
||||
// so a single review picked up 389 files.
|
||||
{"node_modules at root", "node_modules/left-pad/index.js", true},
|
||||
{"node_modules nested in a workspace", "extensions/frontend/node_modules/@babel/core/lib/index.js", true},
|
||||
{"node_modules nested at depth", "a/b/c/d/node_modules/pkg/dist/index.js", true},
|
||||
{"bower_components", "bower_components/jquery/dist/jquery.js", true},
|
||||
{"pnpm store", ".pnpm-store/v3/files/00/abc.js", true},
|
||||
{"yarn unplugged", ".yarn/unplugged/pkg-npm-1.0.0/node_modules/pkg/index.js", true},
|
||||
{"yarn sdks", ".yarn/sdks/typescript/lib/tsc.js", true},
|
||||
{"yarn pnp loader", ".pnp.cjs", true},
|
||||
{"npm lockfile", "package-lock.json", true},
|
||||
{"npm lockfile in a package", "packages/ui/package-lock.json", true},
|
||||
{"pnpm lockfile", "pnpm-lock.yaml", true},
|
||||
{"npm shrinkwrap", "npm-shrinkwrap.json", true},
|
||||
{"minified js", "web/static/app.min.js", true},
|
||||
{"minified css", "web/static/app.min.css", true},
|
||||
{"dist output", "web/dist/main.js", true},
|
||||
{"next build cache", ".next/static/chunks/main.js", true},
|
||||
{"nuxt build output", ".nuxt/dist/client/app.js", true},
|
||||
{"sveltekit generated", ".svelte-kit/generated/root.js", true},
|
||||
{"astro generated types", ".astro/types.d.ts", true},
|
||||
{"turbo cache", ".turbo/daemon/log.json", true},
|
||||
{"angular cli cache", ".angular/cache/17.0.0/vite/deps/index.js", true},
|
||||
{"parcel cache", ".parcel-cache/index.js", true},
|
||||
{"docusaurus build cache", ".docusaurus/registry.js", true},
|
||||
{"vendored go", "api/vendor/github.com/pkg/errors/errors.go", true},
|
||||
{"bundler vendored gems", ".bundle/ruby/3.2.0/gems/rails/lib/rails.rb", true},
|
||||
{"cargo or maven target", "target/debug/build/foo/out.rs", true},
|
||||
{"gradle cache", ".gradle/caches/modules-2/init.gradle.kts", true},
|
||||
{"python bytecode cache", "svc/__pycache__/mod.py", true},
|
||||
{"python dot venv", ".venv/lib/python3.11/site-packages/foo/bar.py", true},
|
||||
{"python bare venv", "venv/lib/python3.11/site-packages/foo/bar.py", true},
|
||||
{"python site-packages anywhere", "env/lib/site-packages/requests/api.py", true},
|
||||
{"python egg-info", "src/mypkg.egg-info/PKG-INFO", true},
|
||||
{"tox env", ".tox/py311/lib/python3.11/x.py", true},
|
||||
{"mypy cache", ".mypy_cache/3.11/foo.json", true},
|
||||
{"pytest cache", ".pytest_cache/v/cache/lastfailed", true},
|
||||
{"ruff cache", ".ruff_cache/content.json", true},
|
||||
{"cocoapods", "ios/Pods/Firebase/Core/FIRApp.m", true},
|
||||
{"carthage", "Carthage/Build/iOS/Alamofire.swift", true},
|
||||
{"swiftpm build dir", ".build/checkouts/swift-nio/Sources/NIO/Channel.swift", true},
|
||||
{"dotnet obj", "src/Api/obj/Debug/net8.0/Api.AssemblyInfo.cs", true},
|
||||
{"dart tool", ".dart_tool/package_config.json", true},
|
||||
{"terraform modules", "infra/.terraform/modules/vpc/main.tf", true},
|
||||
{"terraform lockfile", "infra/.terraform.lock.hcl", true},
|
||||
{"haskell stack work", ".stack-work/dist/x/Main.hs", true},
|
||||
{"coverage report", "coverage/lcov-report/index.js", true},
|
||||
|
||||
// Lookalikes: a directory pattern must match a whole path segment, not
|
||||
// a prefix of one, or these ordinary sources disappear from review.
|
||||
{"distribution is not dist", "src/distribution/index.js", false},
|
||||
{"objects is not obj", "src/objects/model.ts", false},
|
||||
{"targeting is not target", "app/targeting/rules.go", false},
|
||||
{"vendors is not vendor", "src/vendors/stripe.ts", false},
|
||||
{"podsmith is not Pods", "ios/podsmith/Helper.swift", false},
|
||||
{"venvironment is not venv", "tools/venvironment/setup.py", false},
|
||||
{"coverages is not coverage", "app/coverages/report.ts", false},
|
||||
{"coverage as a domain dir is reviewed", "src/coverage/plan.ts", false},
|
||||
{"node_modules in a filename", "src/node_modules_helper.ts", false},
|
||||
{"min in a filename is not minified", "src/minified.ts", false},
|
||||
{"lockfile lookalike", "src/package-lock-utils.ts", false},
|
||||
// #1494 leaves these two out on purpose: plenty of projects keep
|
||||
// hand-written sources in them, including this repository's bin/ocr.js.
|
||||
{"bin is reviewed", "bin/ocr.js", false},
|
||||
{"build is reviewed", "build/index.js", false},
|
||||
|
||||
// Case insensitive
|
||||
{"case insensitive go", "Foo/Bar_Test.go", true},
|
||||
{"case insensitive java", "com/FooTEST.java", true}, // lowercase → "com/footest.java" matches "**/*test.java"
|
||||
|
||||
@@ -38,7 +38,6 @@
|
||||
"**/*Tests.swift",
|
||||
"**/Tests/**/*.swift",
|
||||
"**/tests/**/*.elm",
|
||||
"**/vendor/**/*.{jsonnet,libsonnet}",
|
||||
"**/test/**/*.zig",
|
||||
"**/*_test.zig",
|
||||
"**/kitex_gen/**/*.go",
|
||||
@@ -55,5 +54,52 @@
|
||||
"**/test/**/*.sol",
|
||||
"**/tests/**/*.sol",
|
||||
"**/test/**/*.vy",
|
||||
"**/tests/**/*.vy"
|
||||
"**/tests/**/*.vy",
|
||||
|
||||
"**/node_modules/**",
|
||||
"**/bower_components/**",
|
||||
"**/.pnpm-store/**",
|
||||
"**/.yarn/{cache,unplugged,releases,sdks,patches}/**",
|
||||
"**/.pnp.cjs",
|
||||
"**/.pnp.loader.mjs",
|
||||
"**/package-lock.json",
|
||||
"**/pnpm-lock.yaml",
|
||||
"**/npm-shrinkwrap.json",
|
||||
"**/*.min.{js,css}",
|
||||
"**/dist/**",
|
||||
"**/.next/**",
|
||||
"**/.nuxt/**",
|
||||
"**/.svelte-kit/**",
|
||||
"**/.astro/**",
|
||||
"**/.turbo/**",
|
||||
"**/.angular/**",
|
||||
"**/.parcel-cache/**",
|
||||
"**/.docusaurus/**",
|
||||
|
||||
"**/vendor/**",
|
||||
"**/.bundle/**",
|
||||
|
||||
"**/target/**",
|
||||
"**/.gradle/**",
|
||||
|
||||
"**/__pycache__/**",
|
||||
"**/.venv/**",
|
||||
"**/venv/**",
|
||||
"**/site-packages/**",
|
||||
"**/*.egg-info/**",
|
||||
"**/.tox/**",
|
||||
"**/.mypy_cache/**",
|
||||
"**/.pytest_cache/**",
|
||||
"**/.ruff_cache/**",
|
||||
|
||||
"**/Pods/**",
|
||||
"**/Carthage/**",
|
||||
"**/.build/**",
|
||||
|
||||
"**/obj/**",
|
||||
"**/.dart_tool/**",
|
||||
"**/.terraform/**",
|
||||
"**/.terraform.lock.hcl",
|
||||
"**/.stack-work/**",
|
||||
"**/coverage/lcov-report/**"
|
||||
]
|
||||
|
||||
@@ -63,7 +63,7 @@ The function returns one of:
|
||||
binary — file is binary
|
||||
user_exclude — matched a pattern in your `exclude` list
|
||||
unsupported_ext — extension is not in supported_file_types.json
|
||||
default_path — matched a built-in test-file exclude pattern
|
||||
default_path — matched a built-in exclude pattern
|
||||
```
|
||||
|
||||
…or empty if the file is kept. `deleted` and `too_large` are **not**
|
||||
@@ -78,16 +78,21 @@ order:
|
||||
matches one, it's kept immediately (returns empty), bypassing the
|
||||
`unsupported_ext` and `default_path` gates below.
|
||||
4. `unsupported_ext` filters by extension allowlist.
|
||||
5. `default_path` is the last gate: it matches built-in **test-file**
|
||||
exclude patterns (`**/*_test.go`, `**/*.test.{js,jsx,ts,tsx}`,
|
||||
`**/__tests__/**`, `**/*_test.py`, `**/*_spec.rb`, `**/*.test.ets`, …).
|
||||
Every pattern is rooted with a `**/` prefix.
|
||||
5. `default_path` is the last gate: it matches built-in exclude patterns,
|
||||
both test files (`**/*_test.go`, `**/__tests__/**`, `**/*_spec.rb`, …)
|
||||
and dependency or build-output directories (`**/node_modules/**`,
|
||||
`**/vendor/**`, `**/target/**`, `**/__pycache__/**`, …). Every pattern
|
||||
is rooted with a `**/` prefix.
|
||||
|
||||
The noisy-directory filtering (`vendor/`, `node_modules/`, `target/`, …)
|
||||
happens earlier, at the diff-provider level, via the
|
||||
`providerDirIgnoreDirs` list in `internal/diff/git.go`. Preview reports these
|
||||
files as `provider_directory`; they never reach the per-file filter, and an
|
||||
`include` rule cannot make them reviewable.
|
||||
The same noisy directories are also filtered earlier, at the diff-provider
|
||||
level, via the `providerDirIgnoreDirs` list in `internal/diff/git.go`. That
|
||||
list matches by path prefix, so it catches only a directory at the
|
||||
**repository root**: `vendor/pkg/x.go` never reaches the per-file filter,
|
||||
while `api/vendor/pkg/x.go` does and is excluded by `default_path` instead.
|
||||
|
||||
The difference is user-visible. Preview reports the first as
|
||||
`provider_directory` and an `include` rule cannot make it reviewable; it
|
||||
reports the second as `default_path`, which an `include` rule can override.
|
||||
|
||||
Run `ocr review --preview` to see the full filter result without spending
|
||||
a token. See [Review Rules](../review-rules/#how-files-are-filtered) for
|
||||
|
||||
@@ -95,9 +95,11 @@ For each diff, OCR asks:
|
||||
5. **`unsupported_ext`** — Is the file extension in the
|
||||
[allowlist](https://github.com/alibaba/open-code-review/blob/main/internal/config/allowlist/supported_file_types.json)?
|
||||
Excluded if not.
|
||||
6. **`default_path`** — Does the path match a built-in test-file exclude
|
||||
pattern (`**/*_test.go`, `**/*.test.{js,jsx,ts,tsx}`, `**/*_spec.rb`,
|
||||
…)? Excluded.
|
||||
6. **`default_path`** — Does the path match a built-in exclude pattern?
|
||||
Excluded. These cover test files (`**/*_test.go`,
|
||||
`**/*.test.{js,jsx,ts,tsx}`, `**/*_spec.rb`, …) and dependency or
|
||||
build-output directories (`**/node_modules/**`, `**/vendor/**`,
|
||||
`**/target/**`, …).
|
||||
|
||||
Files that survive all six gates are sent to the LLM, unless the diff
|
||||
alone exceeds 80% of `max_tokens`: `selectFiles` applies that ceiling
|
||||
@@ -128,8 +130,8 @@ The built-in secret paths are not reviewed (see
|
||||
|
||||
The built-in exclude list (see
|
||||
[`internal/config/allowlist/default_exclude_patterns.json`](https://github.com/alibaba/open-code-review/blob/main/internal/config/allowlist/default_exclude_patterns.json))
|
||||
excludes test files across languages, plus test fixtures, snapshots,
|
||||
generated code, and vendored dependencies:
|
||||
covers two groups. Test files across languages, plus test fixtures,
|
||||
snapshots, and generated code:
|
||||
|
||||
- `**/*_test.go`
|
||||
- `**/src/test/java/**/*.java`
|
||||
@@ -189,10 +191,27 @@ generated code, and vendored dependencies:
|
||||
- `**/test/**/*.vy`
|
||||
- `**/tests/**/*.vy`
|
||||
|
||||
Noisy-directory filtering (`vendor/`, `node_modules/`, `target/`, …)
|
||||
happens earlier, at the diff level in
|
||||
[`internal/diff/git.go`](https://github.com/alibaba/open-code-review/blob/main/internal/diff/git.go),
|
||||
before the per-file filter runs.
|
||||
…and dependency or build-output directories:
|
||||
|
||||
- `**/node_modules/**`
|
||||
- `**/bower_components/**`
|
||||
- `**/vendor/**`
|
||||
- `**/target/**`
|
||||
- `**/dist/**`
|
||||
- `**/__pycache__/**`, `**/.venv/**`, `**/site-packages/**`
|
||||
- `**/Pods/**`, `**/Carthage/**`
|
||||
- `**/.next/**`, `**/.nuxt/**`, `**/.gradle/**`, `**/.terraform/**`, …
|
||||
|
||||
`**/build/**` and `**/bin/**` are deliberately absent: projects keep
|
||||
hand-written sources in both.
|
||||
|
||||
The same noisy directories are also filtered earlier, at the diff level in
|
||||
[`internal/diff/git.go`](https://github.com/alibaba/open-code-review/blob/main/internal/diff/git.go).
|
||||
That list matches by path prefix, so it catches only a directory at the
|
||||
**repository root**. `vendor/pkg/x.go` never reaches the per-file filter and
|
||||
is reported as `provider_directory`; `api/vendor/pkg/x.go` does reach it and
|
||||
is reported as `default_path`. Only the second can be brought back by an
|
||||
`include` rule.
|
||||
|
||||
To **review** a file that matches one of these patterns, add
|
||||
it to the user `include` list — that overrides the default-path gate.
|
||||
|
||||
@@ -58,6 +58,8 @@ default_path — matched a built-in test-file exclude pattern
|
||||
|
||||
ノイズディレクトリのフィルタリング(`vendor/`、`node_modules/`、`target/`……)は、より早い段階、diff-provider 層で、`internal/diff/git.go` の `providerDirIgnoreDirs` リストを通じて発生します。これらのディレクトリの diff は解析されたあと除去され、ファイルごとのフィルターに到達することは決してありません。Preview はこれらのファイルを `provider_directory` として報告します。`include` ルールでこれらをレビュー対象に戻すことはできません。
|
||||
|
||||
このリストはパスの接頭辞で照合するため、対象は**リポジトリルート**のディレクトリだけです。ネストした同名ディレクトリはファイルごとのフィルターに到達し、`default_path` で除外されます。`vendor/pkg/x.go` は `provider_directory`、`api/vendor/pkg/x.go` は `default_path` として報告され、`include` ルールで戻せるのは後者だけです。
|
||||
|
||||
`ocr review --preview` を実行すると、token を消費せずに完全なフィルタリング結果を確認できます。完全なアルゴリズムは[レビュールール](../review-rules/#how-files-are-filtered)を参照してください。
|
||||
|
||||
## セマンティックなファイルグルーピング
|
||||
|
||||
@@ -93,7 +93,7 @@ OCR は [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
|
||||
|
||||
### デフォルトパスの除外
|
||||
|
||||
組み込みの除外リスト([`internal/config/allowlist/default_exclude_patterns.json`](https://github.com/alibaba/open-code-review/blob/main/internal/config/allowlist/default_exclude_patterns.json) を参照)は、各言語のテストファイルに加えて、テスト fixture、スナップショット、生成コード、vendored な依存を除外します:
|
||||
組み込みの除外リスト([`internal/config/allowlist/default_exclude_patterns.json`](https://github.com/alibaba/open-code-review/blob/main/internal/config/allowlist/default_exclude_patterns.json) を参照)は 2 つのグループを対象とします。1 つ目は各言語のテストファイルに加えて、テスト fixture、スナップショット、生成コードです:
|
||||
|
||||
- `**/*_test.go`
|
||||
- `**/src/test/java/**/*.java`
|
||||
@@ -153,9 +153,22 @@ OCR は [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
|
||||
- `**/test/**/*.vy`
|
||||
- `**/tests/**/*.vy`
|
||||
|
||||
ノイズディレクトリのフィルタリング(`vendor/`、`node_modules/`、`target/`……)は、より早い段階、[`internal/diff/git.go`](https://github.com/alibaba/open-code-review/blob/main/internal/diff/git.go) の diff 層で発生し、ファイルごとのフィルタリングより先に実行されます。
|
||||
……および依存関係とビルド出力のディレクトリ:
|
||||
|
||||
これらのパターンに一致するファイルを**レビューする**には、それをユーザー `include` リストに追加してください。それが default-path ゲートを上書きします。
|
||||
- `**/node_modules/**`
|
||||
- `**/bower_components/**`
|
||||
- `**/vendor/**`
|
||||
- `**/target/**`
|
||||
- `**/dist/**`
|
||||
- `**/__pycache__/**`、`**/.venv/**`、`**/site-packages/**`
|
||||
- `**/Pods/**`、`**/Carthage/**`
|
||||
- `**/.next/**`、`**/.nuxt/**`、`**/.gradle/**`、`**/.terraform/**`……
|
||||
|
||||
`**/build/**` と `**/bin/**` は意図的に含めていません。手書きのソースをそこに置くプロジェクトが多いためです。
|
||||
|
||||
同じディレクトリは、より早い [`internal/diff/git.go`](https://github.com/alibaba/open-code-review/blob/main/internal/diff/git.go) の diff 層でもフィルタリングされます。このリストはパスの接頭辞で照合するため、**リポジトリルート**のディレクトリしか捕捉しません。`vendor/pkg/x.go` はファイルごとのフィルタに届かず `provider_directory` として報告され、`api/vendor/pkg/x.go` は届いて `default_path` で除外されます。`include` ルールで戻せるのは後者だけです。
|
||||
|
||||
上記いずれかのパターンに一致するファイルを**レビューする**には、それをユーザー `include` リストに追加してください。それが default-path ゲートを上書きします。
|
||||
|
||||
## ファイルごとのルール解決
|
||||
|
||||
|
||||
@@ -86,6 +86,11 @@ diff 프로바이더 단계에서, `internal/diff/git.go`의 `providerDirIgnoreD
|
||||
떼어 내므로 파일 단위 필터까지 오지 못합니다. Preview는 이 파일들을 `provider_directory`로
|
||||
보고하며, `include` 규칙으로도 이들을 다시 리뷰 대상으로 되돌릴 수 없습니다.
|
||||
|
||||
이 목록은 경로 접두사로 비교하므로 **저장소 루트**의 디렉터리만 대상입니다.
|
||||
중첩된 같은 이름의 디렉터리는 파일 단위 필터까지 도달해 `default_path` 로
|
||||
제외됩니다. `vendor/pkg/x.go` 는 `provider_directory` 로, `api/vendor/pkg/x.go` 는
|
||||
`default_path` 로 보고되며 `include` 규칙으로 되돌릴 수 있는 것은 후자뿐입니다.
|
||||
|
||||
`ocr review --preview`를 돌리면 토큰 한 톨 쓰지 않고 필터 결과 전체를 볼 수
|
||||
있습니다. 알고리즘 전체는
|
||||
[리뷰 규칙](../review-rules/#how-files-are-filtered)을 참고하세요.
|
||||
|
||||
@@ -119,7 +119,7 @@ OCR은 [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublesta
|
||||
|
||||
내장 제외 목록은
|
||||
([`internal/config/allowlist/default_exclude_patterns.json`](https://github.com/alibaba/open-code-review/blob/main/internal/config/allowlist/default_exclude_patterns.json)
|
||||
참고) 여러 언어의 테스트 파일과 테스트 fixture, 스냅샷, 생성 코드, vendored 의존성을 제외합니다.
|
||||
참고) 두 그룹을 다룹니다. 첫 번째는 여러 언어의 테스트 파일과 테스트 fixture, 스냅샷, 생성 코드입니다.
|
||||
|
||||
- `**/*_test.go`
|
||||
- `**/src/test/java/**/*.java`
|
||||
@@ -179,12 +179,28 @@ OCR은 [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublesta
|
||||
- `**/test/**/*.vy`
|
||||
- `**/tests/**/*.vy`
|
||||
|
||||
잡음이 많은 디렉터리(`vendor/`, `node_modules/`, `target/` 등)를 걸러내는 일은 더
|
||||
앞에서, 파일별 필터가 돌기 전
|
||||
[`internal/diff/git.go`](https://github.com/alibaba/open-code-review/blob/main/internal/diff/git.go)의
|
||||
diff 단계에서 일어납니다.
|
||||
…그리고 의존성 디렉터리와 빌드 산출물 디렉터리:
|
||||
|
||||
이런 패턴에 걸리는 파일을 **리뷰하고 싶다면** 사용자 `include` 목록에
|
||||
- `**/node_modules/**`
|
||||
- `**/bower_components/**`
|
||||
- `**/vendor/**`
|
||||
- `**/target/**`
|
||||
- `**/dist/**`
|
||||
- `**/__pycache__/**`, `**/.venv/**`, `**/site-packages/**`
|
||||
- `**/Pods/**`, `**/Carthage/**`
|
||||
- `**/.next/**`, `**/.nuxt/**`, `**/.gradle/**`, `**/.terraform/**`, …
|
||||
|
||||
`**/build/**` 와 `**/bin/**` 은 의도적으로 빠져 있습니다. 많은 프로젝트가 손으로
|
||||
작성한 소스를 그 안에 두기 때문입니다.
|
||||
|
||||
같은 디렉터리들은 더 앞선 diff 단계
|
||||
[`internal/diff/git.go`](https://github.com/alibaba/open-code-review/blob/main/internal/diff/git.go) 에서도 걸러집니다.
|
||||
이 목록은 경로 접두사로 비교하므로 **저장소 루트**의 디렉터리만 잡습니다.
|
||||
`vendor/pkg/x.go` 는 파일별 필터까지 오지 않고 `provider_directory` 로
|
||||
보고되고, `api/vendor/pkg/x.go` 는 도달해서 `default_path` 로 제외됩니다.
|
||||
`include` 규칙으로 되돌릴 수 있는 것은 후자뿐입니다.
|
||||
|
||||
위 패턴 중 하나에 걸리는 파일을 **리뷰하고 싶다면** 사용자 `include` 목록에
|
||||
넣으세요. `include`가 기본 경로 관문을 덮어씁니다.
|
||||
|
||||
## 파일별 규칙 해석 {#rule-resolution-per-file}
|
||||
|
||||
@@ -101,6 +101,12 @@ default_path — совпадение со встроенным шаблон
|
||||
diff удаляются до фильтрации отдельных файлов. Preview сообщает об этих файлах как
|
||||
о `provider_directory`; правило `include` не может вернуть их для проверки.
|
||||
|
||||
Список сопоставляется по префиксу пути, поэтому охватывает только каталоги в
|
||||
**корне репозитория**. Вложенные каталоги с теми же именами доходят до файлового
|
||||
фильтра и исключаются как `default_path`: `vendor/pkg/x.go` отмечается как
|
||||
`provider_directory`, а `api/vendor/pkg/x.go` — как `default_path`, и вернуть
|
||||
правилом `include` можно только второй.
|
||||
|
||||
Чтобы увидеть полный результат фильтрации, не потратив ни одного токена,
|
||||
запустите `ocr review --preview`. Полный алгоритм описан в разделе
|
||||
[Правила ревью](../review-rules/#how-files-are-filtered).
|
||||
|
||||
@@ -127,8 +127,8 @@ OCR использует [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com
|
||||
|
||||
Встроенный список исключений (см.
|
||||
[`internal/config/allowlist/default_exclude_patterns.json`](https://github.com/alibaba/open-code-review/blob/main/internal/config/allowlist/default_exclude_patterns.json))
|
||||
исключает тестовые файлы разных языков, а также фикстуры, снапшоты,
|
||||
сгенерированный код и vendored-зависимости:
|
||||
охватывает две группы. Первая — тестовые файлы разных языков, а также
|
||||
фикстуры, снапшоты и сгенерированный код:
|
||||
|
||||
- `**/*_test.go`
|
||||
- `**/src/test/java/**/*.java`
|
||||
@@ -188,10 +188,27 @@ OCR использует [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com
|
||||
- `**/test/**/*.vy`
|
||||
- `**/tests/**/*.vy`
|
||||
|
||||
Фильтрация шумных каталогов (`vendor/`, `node_modules/`, `target/`, …)
|
||||
происходит раньше, на уровне diff в
|
||||
[`internal/diff/git.go`](https://github.com/alibaba/open-code-review/blob/main/internal/diff/git.go),
|
||||
до запуска попереходного файлового фильтра.
|
||||
…и каталоги зависимостей и сборки:
|
||||
|
||||
- `**/node_modules/**`
|
||||
- `**/bower_components/**`
|
||||
- `**/vendor/**`
|
||||
- `**/target/**`
|
||||
- `**/dist/**`
|
||||
- `**/__pycache__/**`, `**/.venv/**`, `**/site-packages/**`
|
||||
- `**/Pods/**`, `**/Carthage/**`
|
||||
- `**/.next/**`, `**/.nuxt/**`, `**/.gradle/**`, `**/.terraform/**`, …
|
||||
|
||||
`**/build/**` и `**/bin/**` намеренно отсутствуют: во многих проектах в них
|
||||
лежат написанные вручную исходники.
|
||||
|
||||
Те же шумные каталоги фильтруются и раньше, на уровне diff в
|
||||
[`internal/diff/git.go`](https://github.com/alibaba/open-code-review/blob/main/internal/diff/git.go).
|
||||
Этот список сопоставляется по префиксу пути, поэтому ловит каталог только в
|
||||
**корне репозитория**: `vendor/pkg/x.go` не доходит до файлового фильтра и
|
||||
отмечается как `provider_directory`, а `api/vendor/pkg/x.go` доходит и
|
||||
исключается как `default_path`. Вернуть правилом `include` можно только
|
||||
второй.
|
||||
|
||||
Чтобы **отревьюить** файл, совпадающий с одним из этих шаблонов,
|
||||
добавьте его в пользовательский список `include` — это переопределяет
|
||||
|
||||
@@ -79,6 +79,11 @@ default_path — matched a built-in test-file exclude pattern
|
||||
列表——这些目录的 diff 被解析后即被剔除,永远不会到达 per-file 过滤器。
|
||||
Preview 将这些文件报告为 `provider_directory`;`include` 规则无法让它们变为可评审。
|
||||
|
||||
该列表按路径前缀匹配,因此只覆盖**仓库根目录**下的目录。嵌套的同名目录会进入
|
||||
per-file 过滤,并由 `default_path` 排除:`vendor/pkg/x.go` 报告为
|
||||
`provider_directory`,而 `api/vendor/pkg/x.go` 报告为 `default_path`,只有后者能被
|
||||
`include` 规则重新纳入。
|
||||
|
||||
运行 `ocr review --preview` 可不花 token 查看完整过滤结果。完整算法见
|
||||
[评审规则](../review-rules/#how-files-are-filtered)。
|
||||
|
||||
|
||||
@@ -111,7 +111,7 @@ OCR 用 [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
|
||||
|
||||
内置排除列表(见
|
||||
[`internal/config/allowlist/default_exclude_patterns.json`](https://github.com/alibaba/open-code-review/blob/main/internal/config/allowlist/default_exclude_patterns.json))
|
||||
会排除各语言的测试文件,以及测试夹具、快照、生成代码与 vendored 依赖:
|
||||
涵盖两组。第一组是各语言的测试文件,以及测试夹具、快照与生成代码:
|
||||
|
||||
- `**/*_test.go`
|
||||
- `**/src/test/java/**/*.java`
|
||||
@@ -171,11 +171,27 @@ OCR 用 [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
|
||||
- `**/test/**/*.vy`
|
||||
- `**/tests/**/*.vy`
|
||||
|
||||
噪声目录过滤(`vendor/`、`node_modules/`、`target/`……)发生在更早的阶段,位于
|
||||
[`internal/diff/git.go`](https://github.com/alibaba/open-code-review/blob/main/internal/diff/git.go)
|
||||
的 diff 层,先于 per-file 过滤运行。
|
||||
……以及依赖目录和构建产物目录:
|
||||
|
||||
要**评审**一个匹配这些模式的文件,把它加入用户 `include` 列表——那会
|
||||
- `**/node_modules/**`
|
||||
- `**/bower_components/**`
|
||||
- `**/vendor/**`
|
||||
- `**/target/**`
|
||||
- `**/dist/**`
|
||||
- `**/__pycache__/**`, `**/.venv/**`, `**/site-packages/**`
|
||||
- `**/Pods/**`, `**/Carthage/**`
|
||||
- `**/.next/**`, `**/.nuxt/**`, `**/.gradle/**`, `**/.terraform/**`, …
|
||||
|
||||
`**/build/**` 和 `**/bin/**` 有意不在其中:很多项目会把手写源码放在这两个目录下。
|
||||
|
||||
同样这些噪声目录在更早的 diff 层也会被过滤,位于
|
||||
[`internal/diff/git.go`](https://github.com/alibaba/open-code-review/blob/main/internal/diff/git.go)。
|
||||
该列表按路径前缀匹配,因此只能命中**仓库根目录**下的目录:`vendor/pkg/x.go`
|
||||
根本不会进入 per-file 过滤,会被报告为 `provider_directory`;而
|
||||
`api/vendor/pkg/x.go` 会进入,并由 `default_path` 排除。只有后者可以用
|
||||
`include` 规则重新纳入评审。
|
||||
|
||||
要**评审**一个匹配上述任一模式的文件,把它加入用户 `include` 列表——那会
|
||||
覆盖 default-path 门。
|
||||
|
||||
## 每文件的规则解析
|
||||
|
||||
Reference in New Issue
Block a user