Skip to content

Fix documentation inaccuracies: endpoint count, scope table, coverage claim#12

Merged
ronaldvdmeer merged 2 commits intomainfrom
docs/audit-and-corrections
Mar 29, 2026
Merged

Fix documentation inaccuracies: endpoint count, scope table, coverage claim#12
ronaldvdmeer merged 2 commits intomainfrom
docs/audit-and-corrections

Conversation

@ronaldvdmeer
Copy link
Copy Markdown
Owner

Summary

Corrects inaccurate counts, missing scope entries, and a misleading constant description found during a pre-release documentation audit.

Details

README.md:

  • Endpoint count corrected: 15 → 23 (all methods were already implemented, the README was never updated)
  • Feature description updated to name all authenticated endpoint categories (notifications, clones, contacts, calendar, loyalty, killmails were missing)
  • Test coverage claim corrected: "100%" → "≥98%" (actual: 98.04%)
  • Endpoint reference link updated: "all 15 methods" → "all 23 methods"

docs/authentication.md:

  • Scope table was missing 7 entries added over the last three PRs:
    • esi-characters.read_notifications.v1async_get_notifications()
    • esi-clones.read_clones.v1async_get_clones()
    • esi-clones.read_implants.v1async_get_implants()
    • esi-characters.read_contacts.v1async_get_contacts()
    • esi-calendar.read_calendar_events.v1async_get_calendar()
    • esi-characters.read_loyalty.v1async_get_loyalty_points()
    • esi-killmails.read_killmails.v1async_get_killmails()
  • async_get_wallet_journal() now listed alongside async_get_wallet_balance() in the esi-wallet.read_character_wallet.v1 row (they share the same scope)
  • "exports the full set" corrected to "exports a recommended baseline set" — DEFAULT_SCOPES is intentionally a subset of all scopes, not a complete listing

Testing

172 unit tests pass, 98.04% coverage. No code changes — docs only.

Integration testing

No integration test changes required — docs-only PR.

Documentation

This PR is the documentation change.

README:
- Correct endpoint count from 15 to 23
- Update feature description to list all endpoint categories
  (notifications, clones, contacts, calendar, loyalty, killmails were missing)
- Correct test coverage claim from '100%' to '>=98%'
- Update endpoint reference link to reflect 23 methods

docs/authentication.md:
- Add 7 missing scopes to the required OAuth scope table:
  read_notifications, read_clones, read_implants, read_contacts,
  read_calendar_events, read_loyalty, read_killmails
- Note that wallet_journal shares esi-wallet.read_character_wallet.v1
  with wallet_balance
- Correct 'exports the full set' to 'exports a recommended baseline set'
  (DEFAULT_SCOPES is intentionally a subset, not the complete scope list)
Copilot AI review requested due to automatic review settings March 29, 2026 19:20
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Documentation-only update to align the published docs with the library’s already-implemented endpoint surface and auth scope requirements, correcting several inaccuracies found during audit.

Changes:

  • Updates README endpoint count/reference (15 → 23) and adjusts the test coverage claim to reflect current coverage.
  • Expands docs/authentication.md scope table to include missing scopes/method mappings and clarifies DEFAULT_SCOPES as a baseline set.
  • Adds async_get_wallet_journal() to the wallet scope row (shared scope with wallet balance).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
docs/authentication.md Adds missing scope rows, updates wallet scope row to include wallet journal, and clarifies DEFAULT_SCOPES description.
README.md Updates feature bullets and docs links to reflect 23 endpoints and adjusts stated test coverage.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

README.md Outdated
- **Fully async** — built on [aiohttp](https://docs.aiohttp.org/)
- **Typed models** — all API responses are frozen dataclasses with full type annotations
- **15 endpoints** — public (server, character, corporation, universe) and authenticated (wallet, skills, location, industry, market, mail, fatigue)
- **23 endpoints** — public (server, character, corporation, universe) and authenticated (wallet, skills, location, industry, market, mail, notifications, clones, contacts, calendar, loyalty, killmails)
Copy link

Copilot AI Mar 29, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The authenticated endpoint category list drops “fatigue”, but the client still provides async_get_jump_fatigue() (and it’s listed in the scopes table). To keep the README feature summary accurate, add “fatigue” back to the authenticated categories list.

Suggested change
- **23 endpoints** — public (server, character, corporation, universe) and authenticated (wallet, skills, location, industry, market, mail, notifications, clones, contacts, calendar, loyalty, killmails)
- **23 endpoints** — public (server, character, corporation, universe) and authenticated (wallet, skills, location, industry, market, mail, notifications, clones, fatigue, contacts, calendar, loyalty, killmails)

Copilot uses AI. Check for mistakes.
- endpoints.md: expand Pagination section with table of paginated methods
  and page sizes; add Client utilities section documenting clear_etag_cache();
  add Request caching section explaining TTL and ETag layers with examples
- quickstart.md: add Automatic pagination and Request caching sections
- error-handling.md: replace duplicated caching content with cross-reference
  to endpoints.md#request-caching
- README.md: restore 'fatigue' to authenticated endpoint category list
  (addresses Copilot review comment)
@ronaldvdmeer ronaldvdmeer merged commit 4f95113 into main Mar 29, 2026
6 checks passed
@ronaldvdmeer ronaldvdmeer deleted the docs/audit-and-corrections branch March 29, 2026 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants