-
Notifications
You must be signed in to change notification settings - Fork 8
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
feat(docs): remove default nx landing page and make docs homepage base route #162
feat(docs): remove default nx landing page and make docs homepage base route #162
Conversation
📝 WalkthroughWalkthroughThe changes in this pull request involve several key updates to the Next.js application configuration and structure. A new Changes
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
✅ Deploy Preview for website-cuhacking ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for docs-cuhacking ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
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
🧹 Outside diff range and nitpick comments (3)
apps/docs/src/app/(docs)/source.ts (1)
Line range hint
1-68
: Consider improving code organization and documentation.While the current implementation is functional, consider the following suggestions to enhance code maintainability and readability:
Extract the
pages
array to a separate configuration file (e.g.,pageConfig.ts
). This separation of concerns will make it easier to manage and update the page structure.Add JSDoc comments to the main exported functions and objects for better documentation. This will help other developers understand the purpose and usage of each element.
Here's an example of how you could implement these suggestions:
- Create a new file
pageConfig.ts
:import { GitPullRequestCreateArrow as ContributionGuidelinesIcon, Dock as HomeIcon, BookOpenText as KnowledgeBaseIcon, Library as LibrariesIcon, Layers as ToolsIcon, } from 'lucide-react' export const pages = [ { title: 'Home', description: 'Leave none behind regardless of our development cost.', url: '', icon: HomeIcon, }, // ... (rest of the pages array) ]
- Update the current file with JSDoc comments and import the
pages
array:import { createMDXSource } from 'fumadocs-mdx' import { loader } from 'fumadocs-core/source' import { createElement } from 'react' import { icons } from 'lucide-react' import { map } from '../../../.map' import { pages } from './pageConfig' /** * Configures and initializes the documentation loader. * @param {Object} options - Loader configuration options. * @returns {Object} An object containing getPage, getPages, and pageTree functions. */ export const { getPage, getPages, pageTree } = loader({ baseUrl: '/', rootDir: 'docs', icon(icon) { if (!icon) { return } if (icon in icons) return createElement(icons[icon as keyof typeof icons]) }, source: createMDXSource(map), })These changes will improve the overall structure and readability of your code.
apps/docs/src/app/(docs)/[[...slug]]/page.tsx (1)
Line range hint
1-91
: Summary of changes and potential impactThe modifications in this file are focused on updating path structures for documentation content and GitHub edit links. These changes align with the PR objectives of modifying the base URL structure. However, the introduction of parentheses in the paths (e.g.,
(docs)
) is noteworthy and may have specific implications in Next.js routing or the file system.Key points to consider:
- Ensure that these path changes are consistent across the entire project.
- Verify that the new directory structure (
src/content/(docs)/
andapps/(docs)/
) exists and contains the expected files.- Test the functionality of accessing documentation content and the "Edit on GitHub" links to confirm they work as expected with the new structure.
These changes, while seemingly minor, could have a significant impact on how documentation is accessed and managed. Thorough testing is recommended to ensure all affected functionalities work correctly with the new path structure.
apps/docs/src/content/docs/index.mdx (1)
Line range hint
70-74
: Consider addressing the TODO items.There are some TODO items listed under the Carleton Open-Source Student Community section:
- Add project demos, screenshots, and descriptions.
- Add auto-generated contributor profiles.
Addressing these items would enhance the documentation and provide more value to the users.
Would you like assistance in implementing these TODO items or creating GitHub issues to track them?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
apps/docs/src/app/(docs)/favicon.ico
is excluded by!**/*.ico
📒 Files selected for processing (9)
- apps/docs/next.config.mjs (1 hunks)
- apps/docs/src/app/(docs)/[[...slug]]/page.tsx (1 hunks)
- apps/docs/src/app/(docs)/layout.config.tsx (2 hunks)
- apps/docs/src/app/(docs)/source.ts (1 hunks)
- apps/docs/src/app/api/search/route.ts (1 hunks)
- apps/docs/src/app/global.css (0 hunks)
- apps/docs/src/app/layout.tsx (0 hunks)
- apps/docs/src/app/page.tsx (0 hunks)
- apps/docs/src/content/docs/index.mdx (1 hunks)
💤 Files with no reviewable changes (3)
- apps/docs/src/app/global.css
- apps/docs/src/app/layout.tsx
- apps/docs/src/app/page.tsx
🧰 Additional context used
🔇 Additional comments (6)
apps/docs/src/app/api/search/route.ts (1)
2-2
: Verify import path change across the codebaseThe updated import path looks correct and aligns with the mentioned directory restructuring. However, please ensure that similar import statements are updated consistently across the codebase.
Could you clarify the purpose of the parentheses in the new path
'../../../app/(docs)/source'
? This seems to be a specific Next.js routing convention, but it would be helpful to confirm its intended use.To verify the consistency of this change, please run the following script:
✅ Verification successful
Import path change verified successfully
All imports from
'../../../app/docs/source'
have been updated to'../../../app/(docs)/source'
. No remaining imports use the old path.Could you please clarify the purpose of the parentheses in the new path
'../../../app/(docs)/source'
? This seems to follow a specific convention, such as Next.js routing, but confirmation would be helpful.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining imports from the old path structure # Test: Search for imports from the old path. Expect: No results. rg --type typescript "from '.*app/docs/source'" apps/docs/src # Test: Search for imports from the new path. Expect: Multiple results including this file. rg --type typescript "from '.*app/\(docs\)/source'" apps/docs/srcLength of output: 213
Script:
#!/bin/bash # Description: Check for any remaining imports from the old path structure # Test: Search for imports from the old path. Expect: No results. rg "from '\\..*app/docs/source'" apps/docs/src --glob '*.ts' --glob '*.tsx' # Test: Search for imports from the new path. Expect: Multiple results including this file. rg "from '\\..*app/\(docs\)/source'" apps/docs/src --glob '*.ts' --glob '*.tsx'Length of output: 261
apps/docs/next.config.mjs (1)
19-27
: Approve image configuration with minor suggestions.The addition of the
images
configuration is a good practice for handling remote images securely in Next.js. Here are some observations and suggestions:
- The use of HTTPS protocol only is excellent for security.
- The dynamic hostname configuration with a fallback is a flexible approach.
However, there are a couple of points to consider:
- To resolve the ESLint warning, consider using the global
process
object:- // eslint-disable-next-line node/prefer-global/process - hostname: process.env.ALLOWED_IMAGE_HOSTNAME || 'avatars.githubusercontent.com', + hostname: globalThis.process.env.ALLOWED_IMAGE_HOSTNAME || 'avatars.githubusercontent.com',
- It would be helpful to add a comment explaining the purpose of
ALLOWED_IMAGE_HOSTNAME
for better maintainability.Let's verify the impact of this change on the codebase:
This script will help identify any potential places where remote images are used, ensuring they comply with the new configuration.
apps/docs/src/app/(docs)/[[...slug]]/page.tsx (1)
20-20
: Verify the new path structure for documentation content.The path for accessing documentation content has been updated to include parentheses around 'docs'. While this change aligns with the PR objectives of modifying the base URL structure, it's important to ensure that this new path correctly reflects the actual directory structure in the project.
Please confirm that:
- The directory structure has been updated to match this new path.
- All documentation files are correctly located in the
src/content/(docs)/
directory.- This change doesn't break any existing functionality for accessing documentation content.
#!/bin/bash # Verify the existence of the new directory structure and content files # Check if the new directory exists if [ -d "src/content/(docs)" ]; then echo "Directory src/content/(docs) exists." # List contents of the directory ls -R src/content/(docs) else echo "Error: Directory src/content/(docs) does not exist." fi # Check for any remaining references to the old path echo "Checking for any remaining references to the old path:" rg "src/content/docs" --type ts --type tsxapps/docs/src/app/(docs)/layout.config.tsx (3)
26-26
: LGTM: Base URL updated correctlyThe change from
'/docs'
to'/'
in thebaseOptions.nav.url
property aligns with the PR objective of simplifying the base URL. This modification removes the redundant/docs
segment, streamlining the navigation structure.
86-86
: LGTM: Sidebar URLs updated consistentlyThe change from
'/docs/${page.url}'
to'/${page.url}'
in thedocsOptions.sidebar
configuration is consistent with the base URL update. This modification ensures that all sidebar links in the documentation follow the new URL structure, removing the redundant/docs
segment.
26-26
: Verify impact on routing and existing linksThe URL structure changes in both
baseOptions
anddocsOptions
are consistent and align with the PR objectives. However, it's crucial to ensure that these modifications don't inadvertently break any existing links or cause routing issues throughout the application.Please run the following script to check for any remaining references to the old URL structure:
If any matches are found, please update them to reflect the new URL structure.
Also applies to: 86-86
@@ -46,7 +46,7 @@ export const pages = [ | |||
] | |||
|
|||
export const { getPage, getPages, pageTree } = loader({ | |||
baseUrl: '/docs', | |||
baseUrl: '/', |
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.
💡 Codebase verification
Residual '/docs' References Detected
The verification script identified several files containing '/docs' references that may need updating to align with the new baseUrl
change:
apps/docs/src/app/docs/source.ts
apps/docs/src/content/docs/index.mdx
apps/docs/src/content/docs/knowledge-base/nextauth.mdx
apps/docs/src/content/docs/knowledge-base/index.mdx
apps/docs/src/content/docs/contribution-guidelines/conventional-commits.mdx
apps/docs/src/app/api/search/route.ts
apps/docs-e2e/src/docs.spec.ts
apps/landing-page/src/pages/homePage/heroSection/SocialsPannel.tsx
Please review and update these references to ensure consistency and prevent potential routing issues.
🔗 Analysis chain
LGTM! The baseUrl
change aligns with the PR objectives.
The modification of baseUrl
from '/docs' to '/' correctly implements the desired change to streamline the documentation URL structure as described in the PR objectives.
To ensure consistency across the codebase, let's verify that there are no remaining references to the old '/docs' base URL:
Please review the results of this search and update any remaining '/docs' references if necessary.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining '/docs' references that might need updating
# Test: Search for '/docs' references in all files.
# Expect: No occurrences of '/docs' that are related to URL routing or base paths.
rg '/docs' --type-not md
Length of output: 16504
const gitHubRepoUrl = packageJson.repository.url.replace(/\.git$/, '') // Remove .git suffix | ||
|
||
const footer = ( | ||
<> | ||
<a | ||
href={`${gitHubRepoUrl}/blob/main/apps/docs/${path}`} | ||
href={`${gitHubRepoUrl}/blob/main/apps/(docs)/${path}`} |
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.
💡 Codebase verification
Issue Found: Incorrect GitHub URL Path
The "Edit on GitHub" link is pointing to a non-existent apps/(docs)/
directory. Based on the repository structure, the correct path should likely be docs/src/app/(docs)/
.
- Update the
href
to:href={`${gitHubRepoUrl}/blob/main/docs/src/app/(docs)/${path}`}
Please verify that the updated path correctly points to the intended files in the repository.
🔗 Analysis chain
LGTM! Verify GitHub repository structure.
The update to the GitHub repository URL for editing the page is consistent with the changes made to the content path. This ensures that the "Edit on GitHub" link will correctly point to the new file location.
To ensure everything is correctly set up, please verify:
- The GitHub repository structure matches this new path (
apps/(docs)/...
). - The "Edit on GitHub" links are functional and lead to the correct files.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the GitHub repository structure and link functionality
# Extract the GitHub repository URL from package.json
REPO_URL=$(jq -r '.repository.url' package.json | sed 's/\.git$//')
# Check the structure of the apps/(docs) directory in the repository
echo "Checking the structure of apps/(docs) directory in the repository:"
gh repo view $REPO_URL --json files --jq '.files[] | select(.path | startswith("apps/(docs)/"))'
# Test a sample "Edit on GitHub" link
SAMPLE_FILE="src/content/(docs)/index.mdx"
if [ -f "$SAMPLE_FILE" ]; then
EDIT_URL="${REPO_URL}/blob/main/apps/(docs)/${SAMPLE_FILE}"
echo "Testing sample 'Edit on GitHub' link:"
echo $EDIT_URL
# Use curl to check if the URL is accessible
curl -s -o /dev/null -w "%{http_code}" $EDIT_URL
else
echo "Sample file $SAMPLE_FILE not found."
fi
Length of output: 1693
Script:
#!/bin/bash
# Verify the existence of the apps/(docs) directory and the sample file
# Extract the GitHub repository URL from package.json
REPO_URL=$(jq -r '.repository.url' package.json | sed 's/\.git$//')
echo "Searching for the 'apps/(docs)' directory in the repository:"
fd "apps/\(docs\)" --type d
echo "Searching for the sample file 'apps/(docs)/index.mdx':"
fd "index.mdx" apps/(docs)/
Length of output: 561
Script:
#!/bin/bash
# Correctly escape parentheses and verify the existence of the apps/(docs) directory and the sample file
# Extract the GitHub repository URL from package.json
REPO_URL=$(jq -r '.repository.url' package.json | sed 's/\.git$//')
echo "Searching for the 'apps/(docs)' directory in the repository:"
fd "apps/\\(docs\\)" --type d
echo "Searching for the sample file 'apps/(docs)/index.mdx':"
fd "index.mdx" "apps/(docs)/"
Length of output: 524
Script:
#!/bin/bash
# List all directories within the apps/ directory to identify the correct 'docs' path
echo "Listing all directories within 'apps/':"
fd . --type d --base-directory apps/
Length of output: 2612
@@ -3,7 +3,7 @@ title: Welcome to the Docs | |||
description: Leave none behind regardless of our development cost. | |||
--- | |||
|
|||
import { pages } from "../../app/docs/source"; | |||
import { pages } from "../../app/(docs)/source"; |
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.
💡 Codebase verification
Old import path still present in apps/hackathon/src/content/docs/index.mdx
.
The import statement from "../../app/docs/source"
remains in apps/hackathon/src/content/docs/index.mdx
. Please update it to from "../../app/(docs)/source"
to ensure consistency across the project.
🔗 Analysis chain
LGTM! Verify consistency across the project.
The updated import path aligns with the PR objective of modifying the base URL for the documentation page. The use of parentheses in (docs)
is a Next.js convention for route grouping, which helps in organizing the project without affecting the URL structure.
To ensure consistency, let's verify that this change has been applied across the project:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of the old import path and verify the new path exists.
# Test 1: Search for any remaining instances of the old import path
echo "Searching for old import path..."
rg --type mdx "from \"../../app/docs/source\""
# Test 2: Verify the existence of the new source directory
echo "Verifying new source directory..."
fd -t d "(docs)/source" apps/docs/src/app
# Test 3: Check for other files that might need similar updates
echo "Checking for other files that might need updates..."
rg --type mdx "from \"../../app/docs/"
Length of output: 451
Script:
#!/bin/bash
# Description: Check for any remaining instances of the old import path and verify the new path exists.
# Test 1: Search for any remaining instances of the old import path
echo "Searching for old import path..."
rg --glob "*.mdx" 'from "../../app/docs/source"'
# Test 2: Verify the existence of the new source directory
echo "Verifying new source directory..."
fd -t d "(docs)/source" apps/docs/src/app
# Test 3: Check for other files that might need similar updates
echo "Checking for other files that might need updates..."
rg --glob "*.mdx" 'from "../../app/docs/'
Length of output: 581
Bypassing branch protection due to the time crunch with this. |
🎉 This issue has been resolved in version 1.4.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
What does this solve
The base URL for the docs page is now
docs.cuhacking.ca
and notdocs.cuhacking.ca/docs
Summary by CodeRabbit
New Features
Improvements
Bug Fixes
Chores