-
-
Notifications
You must be signed in to change notification settings - Fork 29
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
KHasith/docs/overhaul outdated content #167
Conversation
📝 WalkthroughWalkthroughThis pull request introduces several modifications across various documentation files. Key changes include the addition of warning callouts to emphasize the use of specific commands and the clarification of tools used in the project. The structure of the documentation has been reorganized for improved clarity, with updates to installation instructions and the removal or replacement of outdated tools. The overall aim is to enhance user guidance, particularly regarding commit message conventions and tool usage. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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 docs-cuhacking ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Deploy Preview for website-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: 1
🧹 Outside diff range and nitpick comments (10)
apps/docs/src/content/docs/index.mdx (4)
36-37
: Consider using a more reliable GitHub logo source.The current GitHub logo URL from logo.wine might not be stable long-term. Consider using GitHub's official assets or a local image file.
- src="https://www.logo.wine/a/logo/GitHub/GitHub-Icon-White-Dark-Background-Logo.wine.svg" + src="/images/github-mark-white.svg"
Line range hint
89-94
: Enhance TODOs section with more context and specific tasks.The TODOs section could be more helpful with:
- A brief description of why these improvements are needed
- More specific acceptance criteria for each task
- Links to relevant documentation or examples
TODOS: -Add project demos, screenshots, and descriptions. -Add auto-generated contributor profiles. +- Project Showcases + - Add interactive demos of key features + - Include screenshots of user interfaces + - Write compelling project descriptions (see [documentation guidelines](../contributing/docs-guidelines)) + +- Contributor Recognition + - Implement automated profile generation from GitHub data + - Display contribution statistics and achievements + - Highlight featured contributors of the month
Line range hint
115-131
: Resolve duplicate "Carleton Scheduler" project titles.There are two cards with identical titles but different URLs and contributor lists. This could cause confusion for users. Consider:
- Adding distinguishing suffixes to the titles
- Including brief descriptions to highlight the differences between the projects
<Card - title="Carleton Scheduler" + title="Carleton Scheduler (COMP2804 Q&A)" description={ <> + <li>Q&A platform for COMP2804 course</li> <li>Matthew Macrae-Bovell</li> <li>John Lu</li>
Documentation links need cleanup
The extracted URLs reveal syntax issues in the documentation:
- Several URLs have trailing backticks and curly braces (
}
), which are likely template string artifacts- These characters should be removed as they make the links invalid
Affected locations:
https://docs.github.com/en/issues/planning-and-tracking-with-projects/learning-about-projects/about-projects
}https://www.carletonuniversityscheduler.systems
}https://cuscheduling.com
}https://questions.carletoncomputerscience.ca/comp2804
}https://wiki.scesoc.ca/knowledge-base
}🔗 Analysis chain
Line range hint
1-167
: Documentation changes look good overall!The updates align well with the PR objectives of modernizing content and improving clarity. The shift from Notion to GitHub Projects reflects current best practices.
Let's verify all documentation links are valid:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Extract and verify all URLs in the documentation rg -o 'https?://[^\s\"`]+' | while read -r url; do curl --output /dev/null --silent --head --fail "$url" || echo "Broken link: $url" doneLength of output: 140
Script:
#!/bin/bash # Extract and verify all URLs in the documentation using a different regex pattern rg -o 'https://[^ \)"]+' apps/docs/src/content/docs/index.mdxLength of output: 684
apps/docs/src/content/docs/contribution-guidelines/conventional-commits.mdx (1)
34-36
: LGTM! Consider enhancing the callout with an example.The warning callout is well-placed and provides valuable guidance for new contributors.
Consider enhancing the callout by adding a brief example of what users can expect from the interactive process:
<Callout type="warn" title="Use pnpm commit!"> `pnpm commit` will guide you through writing a conventional commit message. Use it until you are comfortable with conventional commit guidelines. + For example: + ```bash + $ pnpm commit + ? Select the type of change: (Use arrow keys) + ❯ feat: A new feature + fix: A bug fix + docs: Documentation only changes + style: Changes that do not affect the meaning of the code + refactor: A code change that neither fixes a bug nor adds a feature + ``` </Callout>apps/docs/src/content/docs/contribution-guidelines/index.mdx (3)
170-175
: Add context for Playwright installation.While the installation step is correct, it would be helpful to explain why Playwright browsers are needed (e.g., for running end-to-end tests) and whether this step is required for all contributors or only those working on specific aspects of the project.
Line range hint
1-176
: Consider restructuring the prerequisites section.To improve clarity and ensure critical prerequisites aren't missed:
- Consider moving the Windows and monorepo migration warnings into a dedicated "Prerequisites" section before the steps
- Add a checklist of required tools/setups that readers can use to verify they're ready to proceed
Example structure:
## Prerequisites - [ ] Read the Windows-specific requirements (if applicable) - [ ] Understand the monorepo structure - [ ] Install required tools - [ ] Docker - [ ] Node.js v20+ - [ ] pnpm ## Steps (existing steps...)
Line range hint
1-176
: Enhance guidance for new contributors.Consider adding:
- A recommendation on which project new contributors should start with
- A troubleshooting section covering common issues
- Links to project-specific documentation for each project option
apps/docs/src/content/docs/tools-overview/index.mdx (2)
363-382
: Fix alt text and consider enhancing the descriptionThe XState card has an incorrect alt text for its icon. Additionally, consider enhancing the description with a practical example to help developers understand when to use XState.
Apply this diff to fix the alt text:
<img src="https://www.svgrepo.com/show/354581/xstate.svg" - alt="Redux Logo" + alt="XState Logo" width={24} height={24} />Consider adding a practical example to the description:
description={ <> <li>Powerful and flexible state management library based on finite state machines and statecharts.</li> <li> Strong focus on predictability and control over state transitions, allowing for more reliable and well-defined workflows. </li> <li> Ideal for managing complex or multi-step processes, where each state and transition can be explicitly defined and visualized. </li> <li> Separates business logic from UI, making it framework-agnostic and easy to integrate with React, Vue, and other libraries. </li> + <li> + Example: Perfect for managing form wizards, where users progress through multiple steps with validation and conditional paths. + </li> </> }
363-382
: Enhance tRPC description with key benefitsThe current description is quite brief and misses key benefits that would help developers understand why tRPC was chosen.
Consider expanding the description:
description={ <> - <li>End-to-end typesafe API layer for interfacing with databases.</li> + <li>End-to-end typesafe API layer that eliminates the need for traditional REST API or GraphQL boilerplate.</li> + <li>Automatic type inference from your API implementation to your client code.</li> + <li>Excellent TypeScript integration with IDE autocompletion support.</li> + <li>Example: Instead of defining API types manually, tRPC automatically shares types between your backend and frontend.</li> </> }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
apps/docs/src/content/docs/contribution-guidelines/conventional-commits.mdx
(1 hunks)apps/docs/src/content/docs/contribution-guidelines/index.mdx
(1 hunks)apps/docs/src/content/docs/index.mdx
(1 hunks)apps/docs/src/content/docs/knowledge-base/nextauth.mdx
(1 hunks)apps/docs/src/content/docs/tools-overview/index.mdx
(1 hunks)
🔇 Additional comments (5)
apps/docs/src/content/docs/contribution-guidelines/index.mdx (2)
Line range hint 1-176
: Verify alignment with documentation overhaul objectives.
The changes align well with the PR objectives to update outdated content. However, to fully achieve the stated goals:
- Consider adding a "Changes from previous version" section to highlight what's been updated
- Add explicit markers for any remaining outdated content that needs attention
#!/bin/bash
# Search for potential outdated content markers
rg -i "legacy|outdated|deprecated|todo|fixme" apps/docs/src/content/docs/
Line range hint 1-176
: Verify repository URLs and command versions.
There appear to be some inconsistencies:
- The clone commands use
cuhacking/2025
while the PR context mentionscuhacking/hackathon
- Consider pinning versions in installation commands (e.g.,
pnpm i -g pnpm@latest
)
apps/docs/src/content/docs/knowledge-base/nextauth.mdx (1)
Line range hint 1-400
: Consider restructuring or archiving NextAuth documentation
Given that NextAuth is not used in the platform, maintaining extensive NextAuth.js documentation might mislead contributors. This creates a disconnect between documentation and actual implementation.
Consider one of these approaches:
- Move this content to an "Archive" or "Legacy" section if it's kept for historical reference
- Replace this document with documentation for the current authentication implementation
- If NextAuth is being considered for future use, clearly mark this as "Proposed Implementation" or "Future Reference"
Let's check if there are any remaining NextAuth dependencies or implementations:
apps/docs/src/content/docs/tools-overview/index.mdx (2)
363-382
: LGTM! Comprehensive Netlify documentation
The Netlify card provides a clear, comprehensive description of the deployment platform, its features, and benefits. The explanation of deployment safety features and AWS integration is particularly helpful for new contributors.
Line range hint 1-824
: Documentation updates align well with project objectives
The changes successfully modernize the tools overview by:
- Replacing outdated tools with modern alternatives
- Maintaining comprehensive descriptions
- Keeping consistent documentation style
<Callout type="warn" title="We don't use NextAuth!"> | ||
cuHacking opts for a different authentication library (TBD), this page is for reference only. | ||
</Callout> |
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.
Warning needs more context and clarity
The warning callout indicates NextAuth isn't used, but leaves the alternative unspecified ("TBD"). This could create confusion for contributors.
Consider enhancing the warning with:
- The name of the current authentication library in use
- Why this NextAuth documentation is being retained
- When the transition from NextAuth occurred (if applicable)
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.
good stuff! i went ahead to take a look at anything that could be outdated as well and one of the things that is going to be outdated is the pr checklist section. I will update this with my new branch (the one with the templates), but if you want to remove it, you can. Otherwise, good stuff, I'll approve the pr.
🎉 This issue has been resolved in version 1.5.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Why were these changes made?
Our doc site was outdated and external contributors were not able to contribute effectively.
What are the changes?
Summary by CodeRabbit
New Features
pnpm commit
command.Documentation