docs: bug-class audit, UX sweep, roadmap refresh, user-guide entry points#52
Merged
Conversation
Add a new "Bug-Class Audit Protocol" section under Mandatory Rules describing the find-class -> grep -> decide-per-hit -> generalise-test -> note-in-PR loop, with the a7 PR #32 / PR #33 references the protocol mirrors. Also adds the new user-guide entry points (context, version, conditional-plugins) to the Document Map. Drops the "Legacy ... still under migration" framing on pkg/httpmock since it is the canonical unit-test pattern in both a6 and a7. Closes #40. Closes #43.
#14 P2 flagged that context, config, update, and version had no direct entry in the user guide. config (declarative-config.md) and update (auto-update.md) already exist; add the missing two and link them from getting-started.md so they're discoverable. Also add conditional-plugins.md to explain plugins like skywalking that ship with APISIX but are off by default — including how the e2e suite distinguishes "verified in real env" from "conditional / skipped" cases (per the #45 ask). Closes #45.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR extends user and developer documentation by introducing three new command/feature guides (context, version, conditional plugins), refining developer reference materials in AGENTS.md to reflect those additions and add a bug-class audit protocol, and updating the roadmap to mark the first four development phases as complete with linked validation status. ChangesDocumentation and Roadmap Updates
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/user-guide/context.md`:
- Line 72: The doc line saying "You cannot delete the currently active context"
is inaccurate; update the documentation to state that the active context can be
deleted but the CLI will prompt for confirmation ("Context %q is the current
active context. Delete it? (y/N)") and that deletion can be forced with the
--force flag; reference the context delete implementation in delete.go and the
user commands like `a6 context use <other>` and `a6 context delete` when editing
the sentence to describe both the interactive confirmation and the `--force`
override.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 274d8e30-4b41-49ba-9c8e-105dec22db12
📒 Files selected for processing (6)
AGENTS.mddocs/roadmap.mddocs/user-guide/conditional-plugins.mddocs/user-guide/context.mddocs/user-guide/getting-started.mddocs/user-guide/version.md
| a6 context delete staging | ||
| ``` | ||
|
|
||
| You cannot delete the currently active context. Switch to a different one with `a6 context use <other>` first. |
There was a problem hiding this comment.
Inaccurate statement about deleting the active context.
The documentation states "You cannot delete the currently active context," but the actual implementation (shown in the relevant code snippet from delete.go) allows deletion of the active context after a confirmation prompt (or immediately with --force). When deleting the active context, a6 prompts "Context %q is the current active context. Delete it? (y/N)" and proceeds if confirmed.
📝 Proposed correction
-You cannot delete the currently active context. Switch to a different one with `a6 context use <other>` first.
+Deleting the active context requires confirmation. You'll be prompted to confirm unless you pass `--force`. After deletion, if other contexts exist, the first one becomes active automatically.Based on learnings: context command implementation details from pkg/cmd/context/delete/delete.go.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| You cannot delete the currently active context. Switch to a different one with `a6 context use <other>` first. | |
| Deleting the active context requires confirmation. You'll be prompted to confirm unless you pass `--force`. After deletion, if other contexts exist, the first one becomes active automatically. |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@docs/user-guide/context.md` at line 72, The doc line saying "You cannot
delete the currently active context" is inaccurate; update the documentation to
state that the active context can be deleted but the CLI will prompt for
confirmation ("Context %q is the current active context. Delete it? (y/N)") and
that deletion can be forced with the --force flag; reference the context delete
implementation in delete.go and the user commands like `a6 context use <other>`
and `a6 context delete` when editing the sentence to describe both the
interactive confirmation and the `--force` override.
There was a problem hiding this comment.
Pull request overview
This docs-only PR updates contributor guidance and user-guide entry points as part of the GA-readiness documentation cleanup.
Changes:
- Adds a bug-class audit protocol and refreshed document map in
AGENTS.md. - Adds new user guides for contexts, version output, and conditional plugins.
- Refreshes the roadmap status table to mark Phase 4 as complete.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
AGENTS.md |
Updates contributor docs with canonical httpmock wording and bug-class audit protocol. |
docs/roadmap.md |
Marks Phase 4 skill PRs as complete and references GitHub PRs. |
docs/user-guide/context.md |
Adds direct reference documentation for a6 context commands. |
docs/user-guide/version.md |
Adds direct reference documentation for a6 version. |
docs/user-guide/conditional-plugins.md |
Adds guidance for APISIX plugins requiring deployment-side enabling. |
docs/user-guide/getting-started.md |
Expands next-step links to the new and existing user-guide pages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+72
to
+73
| You cannot delete the currently active context. Switch to a different one with `a6 context use <other>` first. | ||
|
|
| |---|---|---|---| | ||
| | `skywalking` | [docs](https://apisix.apache.org/docs/apisix/plugins/skywalking/) | Requires an external SkyWalking OAP collector; off so APISIX doesn't ship a noisy default. | Enabled in this repo's e2e config (`test/e2e/apisix_conf/config.yaml`, `config-docker.yaml`). | | ||
| | `skywalking-logger` | [docs](https://apisix.apache.org/docs/apisix/plugins/skywalking-logger/) | Same external dependency as `skywalking`. | Enabled in this repo's e2e config. | | ||
| | `ai-content-moderation` | [docs](https://apisix.apache.org/docs/apisix/plugins/ai-content-moderation/) | Requires an external moderation backend (e.g. AWS Comprehend). | Not enabled in the e2e config; the matching skill case is `Skip()`-gated. | |
Consolidates the UX findings surfaced during the Run 1 manual walkthrough (#34) into the four classes #41 asks about: doc-vs-code mismatches, silently-dropped args, --output inconsistency, --id semantics, plus an "other UX" bucket. Most findings were already in docs/ga-test-report.md; this document re-groups them under #41's headings and adds widened-sweep findings (--output help text across every <resource> <action> combination, unknown-subcommand silent success). Net: 14 findings catalogued, 4 fixed in PR #51, 10 open for follow-up sub-issues. The "next step" section flags that those issues still need to be filed. Closes #41.
shreemaan-abhishek
changed the title
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
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.
Summary
Four small docs-only sub-issues from #33, grouped because they all touch the same files (
AGENTS.md,docs/) and don't need code review:AGENTS.md.docs/ga-ux-sweep.mdconsolidates findings.pkg/httpmockdoc wording (drop the "Legacy" framing).context,version) + conditional-plugins note.No code touched.
Changes
#40 + #43 (
AGENTS.md)pkg/httpmockis the canonical unit-test pattern in both a6 and a7 (a7'sAGENTS.mddocuments it that way).#41 (
docs/ga-ux-sweep.md, new)Consolidates findings from the Run 1 manual walkthrough under #41's four classes — doc-vs-code mismatches, silently-dropped args,
--outputinconsistency,--idsemantics — plus an "other UX" bucket. Also adds findings from a widened sweep of--helptext across every<resource> <action>combination.Net: 14 findings catalogued, 4 fixed in PR #51, 10 open for follow-up sub-issues. Closing #41 here is contingent on the audit trail; the doc flags that the 10 open findings still need GitHub sub-issues filed (out of scope for this docs PR).
Notable widened-sweep findings:
a6 <resource> <typo>silently shows help with exit=0 (e.g.a6 route deletex --forcesucceeds despite the typo). Centralised fix inpkg/cmd/rootwould catch this for every resource.--outputhelp text confirmed consistently inconsistent:list/deleteshowjson,yaml,table;get/create/updateshowjson,yaml. Matches BUG-6 already tracked in PR fix: GA readiness Run 1 against APISIX 3.16 (#34) #51.#45 part 1 (
docs/roadmap.md)#45 parts 2-4 (
docs/user-guide/)context.md(new) — direct entry point fora6 contextcommands (create / use / list / current / delete), with the per-command flag tables.configuration.mdkeeps the config-file / env-var / precedence reference.version.md(new) — whata6 versionprints and when to use it.conditional-plugins.md(new) — documents plugins that ship with APISIX but are off by default (skywalking, ai-content-moderation, chaitin-waf, etc.), how to enable them, and the three coverage states a6's test suite uses: "verified in real env", "conditional / skipped", "not covered."getting-started.md— expanded "What's Next" to link to the three new files plus existingdeclarative-config.mdandauto-update.md.Test plan
make fmt,make vet,make test— greenmake validate-skills— 40 / 40 valid (regression check)docs/)--helpsweep run against the built binary to verify the--outputand positional-id findingsCloses #40. Closes #41. Closes #43. Closes #45. Part of #33.
Summary by CodeRabbit