Open
Conversation
gardener-prow
bot
added
the
kind/cleanup
✅ Deploy Preview for soon-to-be-gardenercloud ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
gardener-prow
bot
added
cla: yes
Contributor
|
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:
📝 WalkthroughWalkthroughReplaces persona-driven sidebar and menu infrastructure with taxonomy-driven sidebar support: adds a transformPageData hook to populate Changes
Sequence Diagram(s)sequenceDiagram
participant Config as VitePress Config
participant SidebarUtils as Sidebar Utils
participant FS as File System
participant Theme as TaxonomyIndex
Config->>FS: resolve absolute path for index.md
Config->>SidebarUtils: call getTaxonomyChildren(relPath, allSidebars)
SidebarUtils->>SidebarUtils: normalize paths & search sidebars
SidebarUtils->>FS: call hasMarkdownContent(filePath) (optional)
SidebarUtils-->>Config: return taxonomy children array | null
Config->>Config: set page.frontmatter.taxonomyChildren
Theme->>Config: read frontmatter.taxonomyChildren
Theme-->>Theme: render taxonomy list links
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
|
Niklas Klocke left a comment: The footer somehow vanished 🤦🏼♂️ Browser metadata |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vitepress/config.mts:
- Line 25: The index-page check using pageData.relativePath.endsWith('index.md')
is too permissive and matches names like foo-index.md; update the conditional to
ensure the final path segment is exactly "index.md" (for example by testing
path.basename/pageData.relativePath.split('/').pop() === 'index.md' or by
checking endsWith('/index.md') depending on POSIX/Windows needs) so only true
index pages pass; modify the condition around pageData.relativePath in
.vitepress/config.mts accordingly.
In `@Makefile`:
- Around line 115-116: The log message and deletion differ: update the echo or
the rm command so they match; either change the echo line to say "Removing
existing hugo directory..." to reflect that rm -rf hugo removes the whole hugo
tree, or change the removal to rm -rf hugo/content if you only intend to delete
the content subdirectory—adjust the echo and the rm -rf hugo|hugo/content pair
accordingly (refer to the echo "Removing existing hugo/content directory..." and
the rm -rf hugo command).
In `@package.json`:
- Line 9: The package "ts-node" is listed as a runtime dependency but is only a
dev tool; remove it from the dependencies block in package.json and either add
it under devDependencies (e.g., "devDependencies": { "ts-node": "10.9.2" }) or
remove it entirely if unused; after editing package.json run your package
manager to update the lockfile (npm install / yarn install) and ensure no
scripts or source files reference ts-node before committing.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (19)
.vitepress/config.mts.vitepress/data/sidebar.data.ts.vitepress/theme/community-sidebar.ts.vitepress/theme/components/CustomVPMenuLink.vue.vitepress/theme/components/VPMenu.vue.vitepress/theme/components/VPNavBarMenuLink.vue.vitepress/theme/components/VPNavbarMenuGroupWrapper.vue.vitepress/theme/components/VPSidebarGroup.vue.vitepress/theme/docs-sidebar.ts.vitepress/theme/index.ts.vitepress/theme/layouts/EmptyIndexLayout.vue.vitepress/theme/utils/debug-json.ts.vitepress/theme/utils/personaMapping.json.vitepress/theme/utils/sidebar.tsMakefilepackage.jsonpost-processing/part-3.jsvitest.config.tswebsite/documentation/_index.md
💤 Files with no reviewable changes (13)
- .vitepress/theme/community-sidebar.ts
- website/documentation/_index.md
- vitest.config.ts
- .vitepress/data/sidebar.data.ts
- .vitepress/theme/components/CustomVPMenuLink.vue
- .vitepress/theme/index.ts
- .vitepress/theme/utils/personaMapping.json
- .vitepress/theme/components/VPNavBarMenuLink.vue
- .vitepress/theme/docs-sidebar.ts
- .vitepress/theme/utils/debug-json.ts
- .vitepress/theme/components/VPMenu.vue
- .vitepress/theme/components/VPSidebarGroup.vue
- .vitepress/theme/components/VPNavbarMenuGroupWrapper.vue
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.vitepress/theme/index.ts (1)
10-15:⚠️ Potential issue | 🟡 MinorRemove the unused
Window.plausibledeclaration.This interface extension is dead code—plausible is not used anywhere in the codebase. Remove lines 10-15 entirely.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.vitepress/theme/index.ts around lines 10 - 15, Remove the unused global extension that adds the plausible property to Window: delete the "declare global { interface Window { plausible?: (event: string, options?: { props?: Record<string, any> }) => void; } }" block (the Window.plausible declaration) so the dead code is gone; before committing, quickly grep for "plausible" to confirm there are no remaining references.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.vitepress/theme/index.ts:
- Around line 10-15: Remove the unused global extension that adds the plausible
property to Window: delete the "declare global { interface Window { plausible?:
(event: string, options?: { props?: Record<string, any> }) => void; } }" block
(the Window.plausible declaration) so the dead code is gone; before committing,
quickly grep for "plausible" to confirm there are no remaining references.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vitepress/theme/utils/sidebar.ts:
- Around line 283-287: The current concatenation using "base + link" can produce
duplicate or missing slashes; update the join logic used where variables
p/base/link are combined (the block that sets let p = base + link and the
similar blocks at the other occurrences) to normalize slashes: trim trailing
slash from base, trim leading slash from link, then join with a single '/' (and
preserve a leading '/' only if required), then apply the existing logic to
append 'index.md' or '/index.md' and strip the leading slash for comparison.
Ensure you change the three places noted (the let p = base + link block and the
analogous blocks around the p construction at the other occurrences) so all path
joins behave consistently.
- Around line 19-20: Guard the lookup of cleaned[resolvePath] before passing its
items into addTrailingSlashToLinks: ensure cleaned[resolvePath] is defined and
has an items array (e.g., if (cleaned[resolvePath] &&
Array.isArray(cleaned[resolvePath].items)) {
addTrailingSlashToLinks(cleaned[resolvePath].items); }) and skip calling
addTrailingSlashToLinks when the section is missing so sidebar generation won't
throw.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.vitepress/theme/community-sidebar.ts.vitepress/theme/docs-sidebar.ts.vitepress/theme/utils/sidebar.ts
gardener-prow
bot
added
the
needs-rebase
klocke-io
force-pushed
the
feat/remove-persona-filter
branch
from
32ddcb3 to
48f90ad
Compare
gardener-prow
bot
removed
the
needs-rebase
Member
|
/assign |
marc1404
reviewed
Mar 4, 2026
Member
There was a problem hiding this comment.
Awesome work & good riddance! 🧹 ✨ Thanks for structuring the changes in multiple commits; it makes reviewing much easier.
It's indeed impressive to see how much complexity is removed by this simplification in many places. I'm looking forward to this change and in the future to a new structure that statically caters to different personas.
/lgtm
Member
There was a problem hiding this comment.
This is totally subjective, but I don't know how I feel about the new sidebar design yet.
I think I understand the motivation for highlighting directories in bold text. However, it starts to feel a bit heavy once you open a few sections, a few levels deeper.
I don't have a strong preference here, so we can also go with the change and see what others think :)
|
LGTM label has been added. DetailsGit tree hash: 03c174f09f03554da7642e0bd4aed0765a7bf135 |
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
2 participants
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.
How to categorize this PR?
/kind cleanup
What this PR does / why we need it:
#867
To reduce technical complexity and provide an easier ground for reasoning about the content structure, the persona filter should be removed from the documentation landing page
Which issue(s) this PR fixes:
Fixes #
#691
#858
#758
Special notes for your reviewer:
Summary by CodeRabbit
New Features
Refactor
Chores
Style