-
Notifications
You must be signed in to change notification settings - Fork 29
Document feature parity #1180
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Document feature parity #1180
Conversation
WalkthroughAdds a new documentation file at docs/thunderstore_feature_parity.md that compares feature parity between the Legacy and New Thunderstore apps, lists statuses (Complete/Partial/Missing) by domain, includes endpoint notes and PR references, and provides recommendations for prioritization and roadmap planning. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Pre-merge checks (3 passed)✅ Passed checks (3 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. ✨ Finishing touches🧪 Generate unit tests
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (9)
docs/thunderstore_feature_parity.md (9)
3-6
: Fix markdownlint MD003: hr mistaken for setext headingAdd a blank line before the horizontal rule so it’s not parsed as a setext header.
-**Local API docs:** [DOCS](http://localhost.thunderstore/api/docs) ---- +**Local API docs:** [DOCS](http://localhost.thunderstore/api/docs) + +---
32-32
: Polish phrasing: “Get listing list for community” → “Get listings for community”Improves clarity without changing meaning.
-|Get listing list for community| ✅ | ✅ | Complete | GET /listing/{community_id}/ | +|Get listings for community| ✅ | ✅ | Complete | GET /listing/{community_id}/ |
46-51
: Capitalize HTML/README consistentlyMinor copy edits for common acronyms.
-|Get package changelog as html| ✅ | ✅ | Complete | GET /package/{namespace_id}/{package_name}/latest/changelog/ | -|Get package readme as html| ✅ | ✅ | Complete | GET /package/{namespace_id}/{package_name}/latest/readme/ | +|Get package changelog as HTML| ✅ | ✅ | Complete | GET /package/{namespace_id}/{package_name}/latest/changelog/ | +|Get package README as HTML| ✅ | ✅ | Complete | GET /package/{namespace_id}/{package_name}/latest/readme/ | -|Get package version changelog| ✅ | ✅ | Complete | GET /package/{namespace_id}/{package_name}/v/{version_number}/changelog/ | -|Get package version readme| ✅ | ✅ | Complete | GET /package/{namespace_id}/{package_name}/v/{version_number}/readme/ | +|Get package version changelog| ✅ | ✅ | Complete | GET /package/{namespace_id}/{package_name}/v/{version_number}/changelog/ | +|Get package version README| ✅ | ✅ | Complete | GET /package/{namespace_id}/{package_name}/v/{version_number}/readme/ |
42-52
: Parameter placeholder consistency (community_id vs none on Package endpoints)Some Package rows include {community_id} while others don’t. If that mirrors the actual API, add a brief “Parameter conventions” note; if not, align the paths.
Proposed addition near the top of the document (adjust wording as needed):
+### Parameter conventions + +- `{community_id}` is included only for endpoints scoped by community. +- `{namespace_id}` / `{package_name}` are global unless otherwise noted. +- Use slugs/IDs consistently with the API schema.
66-79
: Make PR statuses time-stable“In review” can go stale quickly. Add a “last checked” note for the Partial section so readers know recency.
### Partial + +> Note: Statuses last verified on 2025-09-08. Update when PRs merge.
86-87
: Replace placeholder “Example feature” with real gaps (align with Notes)The Missing table should list actual missing items (e.g., version deletion, manual review) or be omitted until populated.
-| Example feature | ✅ | ❌ | Missing | No API endpoint | +| Delete package version | ✅ | ❌ | Missing | No API endpoint | +| Manual package review | ✅ | ❌ | Missing | No API endpoint |
91-95
: Ensure Notes align with the Missing tableItems called out here (version deletion, manual review) should appear in the Missing section for traceability.
22-39
: Format endpoint paths as code for readabilityConsider wrapping endpoint paths in backticks throughout the tables.
3-5
: Clarify “Local API docs” link for GitHub readersA localhost link won’t work on GitHub. Indicate it’s environment-specific or link to hosted API docs if available.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/thunderstore_feature_parity.md
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/thunderstore_feature_parity.md
3-3: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build docker image
- GitHub Check: Build docker image
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1180 +/- ##
=======================================
Coverage 92.23% 92.23%
=======================================
Files 331 331
Lines 10088 10088
Branches 927 927
=======================================
Hits 9305 9305
Misses 657 657
Partials 126 126 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Refs. None
c6912dd
to
aba2195
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
♻️ Duplicate comments (1)
docs/thunderstore_feature_parity.md (1)
55-64
: Nice: resolved prior typo (“memember” → “member”).The row now reads correctly.
🧹 Nitpick comments (5)
docs/thunderstore_feature_parity.md (5)
32-35
: Tighten phrasing of listing endpoints (reader clarity).Shorten the feature names; keeps the table scannable without changing semantics.
-|Get listing list for community| ✅ | ✅ | Complete | GET /listing/{community_id}/ | -|Get listing with community and namespace| ✅ | ✅ | Complete | GET /listing/{community_id}/{namespace_id}/ | -|Get listing with community, namespace and package name| ✅ | ✅ | Complete | GET /listing/{community_id}/{namespace_id}/{package_name}/ | +|List listings for a community| ✅ | ✅ | Complete | GET /listing/{community_id}/ | +|Get listing by community and namespace| ✅ | ✅ | Complete | GET /listing/{community_id}/{namespace_id}/ | +|Get listing by community, namespace, and package| ✅ | ✅ | Complete | GET /listing/{community_id}/{namespace_id}/{package_name}/ |
70-79
: Date-stamp PR states or automate status to avoid staleness.Add “Checked on 2025-09-11” near the Partial table header, or script a tiny check to pull PR states during docs generation.
-### Partial +### Partial (checked on 2025-09-11)Optionally add a docs-generation step that fetches PR statuses from GitHub.
3-5
: Clarify “Local API docs” environment.If
http://localhost.thunderstore/api/docs
requires dev DNS, note that it’s for local dev and may need hosts config; otherwise users may expecthttp://localhost:xxxx
.-**Local API docs:** [DOCS](http://localhost.thunderstore/api/docs) +**Local API docs (local dev):** [DOCS](http://localhost.thunderstore/api/docs) — requires local dev DNS/hosts.
22-26
: Minor wording nits (consistency).
- “Filter community” → “Get community filters” better matches the Notes path “/filters/”.
- Consider “Get community” → “Get community details.”
-|Filter community| ✅ | ✅ | Complete | GET /community/{community_id}/filters/ | +|Get community filters| ✅ | ✅ | Complete | GET /community/{community_id}/filters/ |
53-64
: Consider adding an “Auth/Permissions” column.Many Team/Package actions require roles or auth. A column helps consumers know prerequisites at a glance.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/thunderstore_feature_parity.md
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
docs/thunderstore_feature_parity.md
3-3: Heading style
Expected: atx; Actual: setext
(MD003, heading-style)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Analyze (javascript)
- GitHub Check: Build docker image
- GitHub Check: Build docker image
🔇 Additional comments (2)
docs/thunderstore_feature_parity.md (2)
44-52
: Community scoping is inconsistent across package endpoints — confirm intended behavior and document rationale.Docs show GET /package/{community_id}/{namespace_id}/{package_name}/permissions/ (docs/thunderstore_feature_parity.md:44) but deprecate/readme/changelog/rate/version endpoints use /package/{namespace_id}/{package_name}/... (docs:45-51) and Cyberstorm implementations/tests expose /api/cyberstorm/package/{namespace}/{package}/... (see django/thunderstore/api/urls.py:121-123; django/thunderstore/api/cyberstorm/tests/*; django/thunderstore/repository/api/v1/tests/test_api_v1.py). If intentional, add a short rationale to the docs; otherwise align routes or add explicit handling.
36-37
: Confirm API path spelling: "dependants" vs "dependents"Docs show GET /listing/{community_id}/{namespace_id}/{package_name}/dependants/ — verify the router actually exposes /dependants/ (if the code uses /dependents/, update docs). Automated search did not locate route definitions; manual verification required.
**Legacy App:** [thunderstore.io](https://thunderstore.io) | ||
**New App:** [new.thunderstore.io](https://new.thunderstore.io) | ||
**Local API docs:** [DOCS](http://localhost.thunderstore/api/docs) | ||
--- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Fix unintended setext heading (markdownlint MD003).
The ---
directly under “Local API docs” turns that line into a setext H2 instead of an HR. Insert a blank line before the dashes (or use three asterisks).
-**Local API docs:** [DOCS](http://localhost.thunderstore/api/docs)
----
+**Local API docs:** [DOCS](http://localhost.thunderstore/api/docs)
+
+---
📝 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.
--- | |
**Local API docs:** [DOCS](http://localhost.thunderstore/api/docs) | |
--- |
🤖 Prompt for AI Agents
In docs/thunderstore_feature_parity.md around line 6, the line with just `---`
immediately following the "Local API docs" heading is being parsed as a setext
H2 instead of a horizontal rule; fix it by inserting a blank line above the
`---` (or alternatively replace the dashes with `***`) so the dashes are
rendered as an HR and satisfy markdownlint MD003.
|Get team info| ✅ | ✅ | Complete | GET /team/{team_id}/ | | ||
|Get list of team members| ✅ | ✅ | Complete | GET /team/{team_id}/member/ | | ||
|Get service accounts for team| ✅ | ✅ | Complete | GET /team/{team_id}/service-account/ | | ||
|Disband a team| ✅ | ✅ | Complete | DELETE /team/{team_name}/disband/ | | ||
|Add a new team member to a team| ✅ | ✅ | Complete | POST /team/{team_name}/member/add/ | | ||
|Update team details| ✅ | ✅ | Complete | PATCH /team/{team_name}/update/ | |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Verification agent
🧩 Analysis chain
Parameter naming mixes team_id and team_name; verify accuracy.
Rows use {team_id}
for GET and {team_name}
for other actions. If the API truly mixes ID vs slug/name, add a note; otherwise align the placeholders.
🏁 Script executed:
#!/bin/bash
# Inspect team routes to confirm param names
rg -nP -C2 'team/\{(team_id|team_name)\}/|member/(add|)|disband|update' --type=py --type=ts --type=tsx
Length of output: 27
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking repository for team route placeholders and related endpoints..."
if command -v rg >/dev/null 2>&1; then
rg -n -C2 'team/\{team_id\}' || true
rg -n -C2 'team/\{team_name\}' || true
rg -n -C2 '/team/\{' || true
rg -n -C2 'disband' || true
rg -n -C2 'member/add' || true
if [ -f docs/thunderstore_feature_parity.md ]; then
echo
echo "docs/thunderstore_feature_parity.md lines 58-63:"
sed -n '58,63p' docs/thunderstore_feature_parity.md || true
else
echo "docs/thunderstore_feature_parity.md not found"
fi
else
echo "rg (ripgrep) not found — falling back to grep"
grep -R -n -C2 -F 'team/{team_id}' || true
grep -R -n -C2 -F 'team/{team_name}' || true
grep -R -n -C2 -F '/team/{' || true
grep -R -n -C2 -F 'disband' || true
grep -R -n -C2 -F 'member/add' || true
if [ -f docs/thunderstore_feature_parity.md ]; then
echo
echo "docs/thunderstore_feature_parity.md lines 58-63:"
sed -n '58,63p' docs/thunderstore_feature_parity.md || true
else
echo "docs/thunderstore_feature_parity.md not found"
fi
fi
Length of output: 50070
Use {team_name} (not {team_id}) in the docs — code uses name/slug.
Code and tests consistently use team_name (e.g. django/thunderstore/api/urls.py uses "team/str:team_name/", views/services accept kwargs/team_name, tests call /api/cyberstorm/team/{team.name}/). Update docs/thunderstore_feature_parity.md (lines 58–63) to replace {team_id} with {team_name} (or add an explicit note only if the API intentionally accepts both).
🤖 Prompt for AI Agents
In docs/thunderstore_feature_parity.md around lines 58 to 63, the API paths
incorrectly use {team_id} but the code and tests use {team_name}/{team.slug};
update the listed endpoints to use {team_name} (e.g. GET /team/{team_name}/, GET
/team/{team_name}/member/, GET /team/{team_name}/service-account/, DELETE
/team/{team_name}/disband/, POST /team/{team_name}/member/add/, PATCH
/team/{team_name}/update/) or add a single clarifying note if the API
intentionally accepts both, ensuring the doc matches the code/tests.
| Leave team | ✅ | ❌ | TODO | No API endpoint | | ||
| Report package | ✅ | ❌ | TODO | No Cyberstorm API endpoint - exists in experimental API | | ||
| Upload new package | ✅ | ❌ | TODO? | How should this be handled? | | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Normalize Status values to match the legend.
Legend uses Complete/Partial/Missing, but rows use “TODO”/“TODO?”. Swap to “Missing” for consistency and clearer scanning.
-| Leave team | ✅ | ❌ | TODO | No API endpoint |
-| Report package | ✅ | ❌ | TODO | No Cyberstorm API endpoint - exists in experimental API |
-| Upload new package | ✅ | ❌ | TODO? | How should this be handled? |
+| Leave team | ✅ | ❌ | Missing | No API endpoint |
+| Report package | ✅ | ❌ | Missing | No Cyberstorm API endpoint — exists in experimental API |
+| Upload new package | ✅ | ❌ | Missing | How should this be handled? |
📝 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.
| Leave team | ✅ | ❌ | TODO | No API endpoint | | |
| Report package | ✅ | ❌ | TODO | No Cyberstorm API endpoint - exists in experimental API | | |
| Upload new package | ✅ | ❌ | TODO? | How should this be handled? | | |
| Leave team | ✅ | ❌ | Missing | No API endpoint | | |
| Report package | ✅ | ❌ | Missing | No Cyberstorm API endpoint — exists in experimental API | | |
| Upload new package | ✅ | ❌ | Missing | How should this be handled? | |
🤖 Prompt for AI Agents
In docs/thunderstore_feature_parity.md around lines 85 to 88, the status cells
use "TODO" and "TODO?" which are inconsistent with the legend's
"Complete/Partial/Missing"; update those table cells to use "Missing" (or
"Partial" where appropriate) so the row values match the legend and scanning is
consistent—replace "TODO" and "TODO?" with "Missing" for the three listed rows
unless any are actually partially implemented, in which case use "Partial".
Refs. None
Summary by CodeRabbit