docs: restructure documentation and optimize README#560
Conversation
- Consolidate usage.md into README Manual section - Add C++ custom filter examples and threading guidelines - Extract filter rules to docs/filter-rules.md (437 lines) - Reduce README from 254 to 193 lines (-24%) - Add Key Classes table and GL threading warnings - Move contributing/release docs to .github/ - Create AI instruction files (.github/skills/, copilot-instructions.md) - Add .copilotignore to exclude build artifacts BREAKING: docs/usage.md removed (content merged into README)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughAdds documentation, contributor guides, CI workflow overhauls, GitHub/Copilot workspace instructions, PR workflow skills, build and filter docs, and a new Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer (push / tag / dispatch)
participant GH as GitHub
participant Runner as Actions Runner
participant Builder as Build System (Gradle / NDK / CMake)
participant ArtifRepo as Artifact Repo
participant ReleaseAPI as GitHub Release API
Dev->>GH: push tag or dispatch workflow
GH->>Runner: trigger .github/workflows/release.yml
Runner->>Runner: extract tag/version, validate vs build.gradle
Runner->>Builder: build demo APK and AAR variants
Builder-->>Runner: produced artifacts + checksums
Runner->>ArtifRepo: clone artifact repo, rsync artifacts, commit & open PR
Runner->>ReleaseAPI: create GitHub Release (conditional)
ReleaseAPI-->>GH: Release created / assets uploaded
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Fix all issues with AI agents
In @.github/copilot-instructions.md:
- Around line 34-39: Update the Documentation list to remove the stale reference
to docs/usage.md and point consumers to the merged location (README.md) instead;
specifically edit the section that currently lists `docs/usage.md` and either
replace that entry with `README.md — Usage, custom filters, and code samples` or
remove the entry entirely so the docs list only includes existing files
(`docs/build.md`, `.github/RELEASE.md`, `.github/CONTRIBUTING.md`).
In @.github/instructions/java.instructions.md:
- Around line 2-3: The applyTo pattern uses pipe separators which GitHub Copilot
expects as comma-separated values; update the applyTo value (the applyTo field
in this file) to replace the pipe character with a comma so it reads as two
comma-separated glob patterns:
library/src/main/java/**/*.java,cgeDemo/src/main/java/**/*.java.
In @.github/RELEASE.md:
- Around line 49-56: Update the wording that currently says "All version parts
must be pure numbers" to clarify that only the core semantic version segments
(X.Y.Z) must be numeric while prerelease suffixes like -beta, -alpha, -rc and
their numeric identifiers are allowed; make it explicit that the tag must match
the versionName in build.gradle exactly (including any prerelease suffix), and
replace the sentence with something like: "The core version segments (X.Y.Z)
must be numeric; prerelease suffixes (for example -beta1, -alpha1, -rc1) are
permitted and the full tag (including any suffix) must exactly match versionName
in build.gradle."
In @.github/workflows/release.yml:
- Around line 309-316: The PR body string uses "\n" inside double quotes so
newlines will be literal; update how PR_BODY is constructed so it contains real
newlines before passing to jq (e.g., build PR_BODY with a C-style expansion or
use printf to inject actual newlines) and ensure the jq --arg body "$PR_BODY"
receives the multiline string; check the symbols PR_BODY and the jq API_JSON
invocation and replace the current double-quoted "\n" usage with a method that
produces real newline characters.
- Around line 241-259: The release flow generates SHA256SUMS.txt and individual
.sha256 files in the "Generate artifact checksums" step but the release upload
only includes *.apk and *.aar; update the release upload step (the job/step that
currently uploads artifacts with patterns '*.apk' and '*.aar') to also include
SHA256SUMS.txt and '*.sha256' so checksum files are published, and ensure the
upload uses the same /tmp/release-artifacts directory or the action's
working-directory so the new files are present when uploading.
- Around line 67-115: The workflow step with id "tag_version" currently injects
the untrusted expression `${{ github.event.inputs.version }}` directly into the
script; instead export that input via an env var (e.g., set env: VERSION: ${{
github.event.inputs.version }}) and reference the shell variable $VERSION inside
the script (use quoting when testing/assigning) so the input is not parsed by
the shell; adjust the block that handles `workflow_dispatch` to read from
$VERSION (and still set TAG_NAME="v$VERSION", BASE_VERSION, IS_MANUAL,
IS_PRERELEASE) and add a strict validation/sanitization check on $VERSION before
use to reject suspicious characters.
🧹 Nitpick comments (2)
docs/filter-rules.md (1)
9-418: Add fenced code block languages to satisfy markdownlint (MD040).If markdownlint is enforced in CI, these fences will fail without a language. Consider adding
text(orjava/cpp/bashwhere appropriate) to each fenced block.Example (apply consistently)
-``` +```text `@method1` <args1> `@method2` <args2> `@method3` <args3></details> </blockquote></details> <details> <summary>README.md (1)</summary><blockquote> `160-166`: **Optional: align strong emphasis style with markdownlint (MD050).** If MD050 is enforced, switch to underscores for strong emphasis to avoid lint noise. <details> <summary>Example</summary> ```diff -**Important**: GL operations must run on the GL thread: +__Important__: GL operations must run on the GL thread:
There was a problem hiding this comment.
Pull request overview
Restructures and consolidates project documentation into dedicated docs/ and .github/ resources, while simplifying the README and improving the release workflow for artifact publishing.
Changes:
- Added comprehensive docs (
docs/build.md,docs/filter-rules.md) and contributor/release guides under.github/. - Simplified
README.mdto focus on quick start + essential usage, linking out to detailed docs. - Reworked
.github/workflows/release.ymlto validate versions, build multiple variants, and publish artifacts/releases.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/filter-rules.md | Adds a full local reference for filter rule strings and syntax. |
| docs/build.md | Adds a dedicated build guide (config flags + build methods). |
| README.md | Replaces long build instructions with a quick-start and links to docs. |
| .github/workflows/release.yml | Overhauls release pipeline (version validation, build variants, publish artifacts). |
| .github/skills/pr-submit/SKILL.md | Adds procedural PR submission workflow documentation. |
| .github/skills/pr-review/SKILL.md | Adds procedural PR review checklist documentation. |
| .github/instructions/java.instructions.md | Updates Copilot instruction targeting patterns for Java files. |
| .github/instructions/cpp.instructions.md | Updates Copilot instruction targeting patterns for C++ files. |
| .github/copilot-instructions.md | Streamlines Copilot guidance and links to repo documentation. |
| .github/RELEASE.md | Adds a local release process guide referencing the workflow. |
| .github/CONTRIBUTING.md | Adds consolidated contributor standards and compatibility rules. |
| .copilotignore | Adds exclusions to reduce Copilot context noise. |
.github/copilot-instructions.md
Outdated
| - For `GLSurfaceView`-based classes, do GL operations on the GL thread via `queueEvent(...)`. | ||
| - `NativeLibraryLoader` controls native library loading order; don’t unconditionally load FFmpeg when video module is disabled. | ||
| - `docs/build.md` — Full build guide and configuration options | ||
| - `docs/usage.md` — API usage, custom filters, and code samples |
There was a problem hiding this comment.
docs/usage.md is referenced here, but the PR description states it was removed and merged into the README. This link will be broken/outdated—update it to point to README.md (Manual section) or to the new dedicated docs that replaced it.
| - `docs/usage.md` — API usage, custom filters, and code samples | |
| - `README.md` (Manual section) — API usage, custom filters, and code samples |
| applyTo: | ||
| - "library/src/main/java/**/*.java" | ||
| - "cgeDemo/src/main/java/**/*.java" | ||
| applyTo: "library/src/main/java/**/*.java|cgeDemo/src/main/java/**/*.java" |
There was a problem hiding this comment.
This applyTo value uses | as an OR separator, but glob matchers typically treat | as a literal character, so the instruction may never apply. Use a YAML list (the previous form) or a single glob that matches both roots (e.g., brace expansion only if confirmed supported by the instruction loader).
| applyTo: "library/src/main/java/**/*.java|cgeDemo/src/main/java/**/*.java" | |
| applyTo: | |
| - "library/src/main/java/**/*.java" | |
| - "cgeDemo/src/main/java/**/*.java" |
| - "library/src/main/jni/cge/**/*.{cpp,h,c}" | ||
| - "library/src/main/jni/custom/**/*.{cpp,h,c}" | ||
| - "library/src/main/jni/interface/**/*.{cpp,h,c}" | ||
| applyTo: "library/src/main/jni/{cge,custom,interface}/**/*.{cpp,h,c}" |
There was a problem hiding this comment.
Brace expansion ({...}) and multi-extension globs (*.{cpp,h,c}) are not universally supported by instruction file glob evaluators. To avoid the instructions silently not applying, prefer the previous YAML list of explicit patterns (one per directory/extension), unless the toolchain is confirmed to support this syntax.
| applyTo: "library/src/main/jni/{cge,custom,interface}/**/*.{cpp,h,c}" | |
| applyTo: | |
| - "library/src/main/jni/cge/**/*.cpp" | |
| - "library/src/main/jni/cge/**/*.h" | |
| - "library/src/main/jni/cge/**/*.c" | |
| - "library/src/main/jni/custom/**/*.cpp" | |
| - "library/src/main/jni/custom/**/*.h" | |
| - "library/src/main/jni/custom/**/*.c" | |
| - "library/src/main/jni/interface/**/*.cpp" | |
| - "library/src/main/jni/interface/**/*.h" | |
| - "library/src/main/jni/interface/**/*.c" |
| inputs: | ||
| version: | ||
| description: 'Version tag (e.g. v1.0.0-test)' | ||
| description: 'Version number (e.g., 3.1.1) - Manual trigger will NOT publish to Release page' | ||
| required: true | ||
| default: 'v0.1.0-test' | ||
| create_release: | ||
| description: 'Whether to create GitHub Release' | ||
| type: boolean | ||
| default: false | ||
| type: string |
There was a problem hiding this comment.
The manual input is described as a version number (no leading v), but later the workflow unconditionally prefixes v to form TAG_NAME. If a user enters v3.1.1, the workflow will produce vv3.1.1 and fail tag-format validation downstream. Add input validation/sanitization (strip a leading v, or explicitly reject values starting with v).
- PR: only run 2 critical configs (down from 16) - NDK-r27c + cmake + ffmpeg + 16kb (latest recommended) - NDK-r26d + ndk-build + no-ffmpeg + 4kb (legacy coverage) - master push: keep all 16 configs for full coverage - Total PR jobs: 35+ → 5 (Windows:1 + macOS:1 + Ubuntu:2 + Lint:1)
…orkflow* - Update macOS build workflow trigger - Update Ubuntu build workflow trigger - Update Windows build workflow trigger This prevents accidental full-matrix builds on branches containing 'workflow' in the name. Only branches starting with 'test_workflow' will trigger the full test matrix.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/release.yml:
- Around line 304-306: The rsync call using the --delete flag ("rsync -av
--delete \"${SOURCE_REPO}/org/wysaid/gpuimage-plus/\"
\"$WORKDIR/org/wysaid/gpuimage-plus/\"") will remove all previously published
versions in WORKDIR/org/wysaid/gpuimage-plus because SOURCE_REPO only contains
this run's artifacts; remove the --delete option to make the sync additive (or,
if the intention is to fully replace the directory, add a clear
comment/documentation above the rsync line stating that publish.sh performs a
full republish and --delete is intentional), ensuring the change references the
rsync invocation, SOURCE_REPO, and the destination path.
- Around line 546-562: The failure notification uses step outputs from
steps.tag_version (steps.tag_version.outputs.version and
steps.tag_version.outputs.tag_name) which may be unset if the tag_version step
failed; update the workflow to guard or default these values: either check that
steps.tag_version.conclusion == 'success' (or use if: steps.tag_version.outcome
== 'success') before emitting the outputs, or substitute a safe fallback like
"unknown" when referencing steps.tag_version.outputs.version and
steps.tag_version.outputs.tag_name in the echo lines so the summary never shows
blank values; adjust the Build failure notification step to implement one of
these approaches referencing the step name tag_version.
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
138-154: Version validation step uses${{ }}expression interpolation directly in shell.Line 141 interpolates
steps.tag_version.outputs.versiondirectly into the script. This value originates from user input (theworkflow_dispatchversion field) and, while it has been regex-validated in the previous step, the pattern is inconsistent with the careful env-var approach established in lines 69-71 for the initial input. The same pattern recurs at lines 235, 288-289, 357-358, and throughout the summary/notification steps.Since the values are regex-constrained to
[0-9a-zA-Z.-], there's no exploitable injection today. However, for defense-in-depth and consistency, consider passing step outputs throughenv:blocks in security-sensitive steps (build commands, git operations).
1 participant
Objective
Restructure project documentation by separating AI-facing instructions from user-facing content, and consolidate scattered information into organized, maintainable documentation.
Changes
📄 Documentation Structure
New files:
.copylotignore(23 lines) — Reduces AI context noise by excluding build artifacts, prebuilt libs, IDE config.github/CONTRIBUTING.md(64 lines) — Consolidated code standards, API stability rules, and dual-build system requirements.github/RELEASE.md(56 lines) — Release process (moved from README).github/skills/pr-submit/SKILL.md(60 lines) — AI skill for PR submission workflow.github/skills/pr-review/SKILL.md(67 lines) — AI skill for PR review checklistdocs/build.md(102 lines) — Full build guide extracted from READMEdocs/filter-rules.md(437 lines) — Comprehensive filter syntax reference (synthesized from English + Chinese wikis, now ~100% complete)Enhanced files:
.github/copilot-instructions.md— Streamlined from 59→52 lines, now indexes all other resourcesREADME.md— Reduced from 254→193 lines (-24%)Removed files:
docs/usage.md— Content merged into README Manual section🎯 Key Improvements
For Users:
For Contributors & AI:
.github/CONTRIBUTING.mdas single source of truth for code standardsDocumentation Quality:
📊 Statistics
🔄 Comparison with origin/master
Design Decisions
Testing
Breaking Changes
docs/usage.mdremoved (content consolidated into README Manual)Related Issues
Resolves documentation organization and completeness issues.
Type of Change: 📚 Documentation
Summary by CodeRabbit
Documentation
Chores