Add docs/ with swagger specs, README TOC, and CLAUDE.md #5
Loading…
Add table
Add a link
Reference in a new issue
No description provided.
Delete branch "1-docs-swagger-specs"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
Summary
docs/(121 endpoints across 10 API groups)docs/api-groups.md— overview table with endpoint countsdocs/dns-endpoints.md— P0 DNS record management reference (6 endpoints, schema, rate limits)docs/auth.md— credential setup guide forsso-keyauthChanges
README.md— full rewrite as TOC linking to docs/, includes mermaid diagrams, usage examples, API coverage tableCLAUDE.md— symlink to README.mddocs/api-groups.md— new, generated from swagger specsdocs/dns-endpoints.md— new, P0 DNS endpoint reference with SDK method mappingdocs/auth.md— new, credential setup and base URL referencedocs/swagger_*.json— all 10 official swagger specsTest Plan
python3 -c "import json; json.load(open('docs/swagger_domains.json'))")api-groups.mdlinks resolve to spec filesReview Checklist
Related Notes
project-godaddy-sdk— project pagePR #5 Review
DOMAIN REVIEW
Tech stack: Documentation-only PR (Markdown, Mermaid diagrams, JSON swagger specs, symlink). No executable code to review for runtime correctness. Domain expertise applied: documentation quality, data accuracy, link integrity, security hygiene.
Endpoint Count Discrepancies (data accuracy)
The README and
docs/api-groups.mdboth claim specific endpoint counts per API group, but spot-checking the actual swagger JSON files reveals mismatches:The claimed total of 121 endpoints appears to be ~97 based on actual path counts in the swagger files. This needs to be reconciled -- either the counting methodology should be documented (e.g., "each HTTP method on a path counts separately") or the numbers corrected.
"10 API groups" vs 11 table rows
The README intro says "10 API groups (121 endpoints)" but the API Coverage table has 11 rows. DNS (6 endpoints) and Domains (65 endpoints) are listed as separate rows but both link to
swagger_domains.json. There is noswagger_dns.json. This is confusing -- DNS is a subset of the Domains spec, not a separate API group. Either:docs/dns-endpoints.md-- SDK method names are speculativeThe SDK Method column (
get_records,replace_records,add_records, etc.) references methods that do not exist yet -- the SDK has no code. This is fine as forward-looking design docs, but should be labeled as "Planned SDK Method" to avoid confusion.docs/api-groups.md-- thin descriptionsEvery group description is just the group name restated ("Abuse API", "Domains API", "Certificates", "Orders"). These add zero information. Either pull actual descriptions from the swagger
info.descriptionfields or remove the Description column.CLAUDE.mdsymlinkSymlink
CLAUDE.md -> README.mdis a clean approach. The symlink mode120000is correct. No missing newline at end of symlink target (expected for symlinks).BLOCKERS
Endpoint count inaccuracy across README.md, docs/api-groups.md, and PR body. The claimed counts do not match the actual swagger specs being committed in the same PR. Documentation that ships with incorrect numbers on day one erodes trust. Fix the counts or document the counting methodology that explains the discrepancy.
"10 API groups" vs 11-row table inconsistency. The README intro and the table contradict each other. Pick one framing and apply it consistently across README.md and api-groups.md.
NITS
docs/api-groups.mddescriptions are redundant. "Abuse API" for the Abuse group adds nothing. Consider pulling the one-liner from each swagger spec'sinfo.descriptionfield, or dropping the column.docs/dns-endpoints.mdSDK Method column. Label as "Planned SDK Method" since no code exists yet.README Usage example references
~/secrets/godaddy/.envin Testing section. This is fine as documentation, but consider adding a note that this path is established bydocs/auth.md-- a reader jumping to Testing without reading Auth will be confused.docs/dns-endpoints.mdrate limit section is terse. "60 requests per minute per API key" -- is this documented in the swagger spec or observed? A source reference would help.docs/api-groups.mduses relative links (swagger_abuse.json). These resolve correctly from withindocs/but will break if the file is viewed from repo root in some renderers. The README table correctly usesdocs/swagger_abuse.json. Consider making api-groups.md consistent -- though this is minor since Forgejo renders relative links correctly within the directory.Missing
Countriesmention. The architecture mermaid diagram lists aCountriesMixinbut the api-groups table lists "Countries" without the Mixin suffix. Consistent naming is a nit but worth aligning.SOP COMPLIANCE
1-docs-swagger-specsreferences issue #1)## Summary-- present, clear## Changes-- present, lists all files## Test Plan-- present with checkboxes## Review Checklist-- present## Related Notes-- present, referencesCloses #1and related issuesyour_key_here,api_key="...").envfiles committed (only documentation references to~/secrets/godaddy/.env)PROCESS OBSERVATIONS
VERDICT: NOT APPROVED
Two blockers must be resolved before merge: (1) endpoint count discrepancies between documentation and actual swagger specs, and (2) the "10 groups" vs 11-row table inconsistency. Both are data accuracy issues in a documentation PR -- the one thing a docs PR must get right is the data.
View command line instructions
Checkout
From your project repository, check out a new branch and test the changes.Merge
Merge the changes and update on Forgejo.Warning: The "Autodetect manual merge" setting is not enabled for this repository, you will have to mark this pull request as manually merged afterwards.