Update docs with extra details #1044
Conversation
|
@chvainickas is attempting to deploy a commit to the Codú Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in the pull request focus on enhancing the Changes
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 (
|
|
Hello @chvainickas, thanks for opening your first Pull Request. The maintainers will review this Pull Request and provide feedback as soon as possible. Keep up the great work! |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (2)
README.md (2)
48-59: Excellent addition of database setup instructions.The new instructions for pushing schema changes and seeding the database are clear and helpful. The references to
package.jsonprovide additional context for users.Consider adding a brief explanation of what these commands do for users who might be unfamiliar with the process. For example:
npm run db:push +# This command applies your Drizzle schema to the database npm run db:seed +# This command populates your database with initial data for testing
Line range hint
127-135: Great addition of Passwordless authentication setup instructions.The new section on setting up Passwordless authentication locally is a valuable addition to the README.
However, the hardcoded AWS region could be problematic for users in different regions.
Consider the following improvements:
- Make the AWS region configurable through an environment variable.
- Update the code to use this environment variable instead of the hardcoded value.
- Add the AWS region to the
.envfile and update the README instructions accordingly.For example:
+AWS_REGION=eu-west-1 # Add this to the .env file + +# Update the code to use process.env.AWS_REGION instead of the hardcoded value + +# Update the README instructions +Check out the example .env file [here](./sample.env) to see how to populate these values, including the AWS_REGION. -**Note: Currenly the AWS region of the SNS service is hardcoded to "eu-west-1" it may be necessary to change this if your SNS service is in a different region** +**Note: Make sure to set the AWS_REGION in your .env file to match your SNS service region.**🧰 Tools
🪛 LanguageTool
[style] ~30-~30: Consider an alternative verb to strengthen your wording.
Context: ...ix any errors you might have. - If you have any issues installing dependencies, che...(IF_YOU_HAVE_THIS_PROBLEM)
README.md
Outdated
| npm install && npm audit fix --force | ||
| ``` | ||
|
|
||
| - The second part of the command should fix any errors you might have. |
There was a problem hiding this comment.
Consider a more cautious approach to vulnerability fixes.
While automatically fixing vulnerabilities can improve security, using npm audit fix --force may lead to unexpected package updates that could potentially break the project.
Consider the following alternatives:
- Use
npm auditfirst to review vulnerabilities, then apply fixes selectively. - If automatic fixes are necessary, use
npm audit fixwithout the--forceflag to apply only non-breaking changes.
Update the instructions to reflect the chosen approach, for example:
-npm install && npm audit fix --force
+npm install
+npm audit
+# Review the audit results and apply fixes as neededCommittable suggestion was skipped due to low confidence.
README.md
Outdated
|
|
||
| ```bash | ||
| npm install | ||
| npm install && npm audit fix --force |
There was a problem hiding this comment.
Hey Edward,
We dont want to add "run audit fix" to the readme, since it will update packages.
Ideally this will be done as a separate PR, and the changes will be pushed to everyone.
I ran this locally myself during the stream, and it is updating sentry, so we will need to do a little investigating on this first. Thanks
There was a problem hiding this comment.
Thank you JP, this is a good point! I will omit this from the final doc
README.md
Outdated
| ```bash | ||
| npm run db:push | ||
| ``` | ||
| The full command can be seen in our [here](/package.json#L16) in our [package.json](/package.json) file |
There was a problem hiding this comment.
Hey Edward. This markdown shows as:
The full command can be seen in our here in our package.json file
Maybe remove "in our here", so it appears as :
The full command can be seen in our package.json file
This applies to the link below also.
There was a problem hiding this comment.
Hi JP, yes, good spot! I'll change that before the final PR. Should be done by today.
|
Hi guys, I've finished the README.md I did not really need to add anything or delete anything, just kind of created more links for anyone that might want to see more under the hood of what is happening and added excerpts to be copied for OAuth, might just quicken the process for people getting set-up. Anyhow, I look forward to your reviews |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
README.md (3)
45-46: Approve addition with minor suggestion.The added note is helpful for users setting up the project. It's good to remind users to ensure the database is running before proceeding.
Consider rephrasing slightly for better clarity:
-**NOTE:** Before proceeding, [make sure your database is running](#database_url). +**NOTE:** Before proceeding, [ensure your database is running](#database_url).
47-53: Approve addition with suggestions for improvement.The added instructions for setting up database tables using Drizzle are valuable. Providing a link to the package.json file for the full command is a good practice for transparency.
Consider the following improvements:
- Fix the grammatical issue:
-6. Setup the tables in the database with Drizzle by running: +6. Set up the tables in the database with Drizzle by running:
- Add a brief explanation of what Drizzle is for newcomers to the project.
- Consider adding a code fence language identifier to the bash code block for better syntax highlighting.
🧰 Tools
🪛 LanguageTool
[grammar] ~47-~47: This sentence should probably be started with a verb instead of the noun ‘Setup’. If not, consider inserting a comma for better clarity.
Context: ...tabase is running](#database_url). 6. Setup the tables in the database with Drizzle...(SENT_START_NN_DT)
Line range hint
97-119: Approve detailed GitHub OAuth setup instructions with minor suggestion.The added instructions for setting up GitHub OAuth are excellent. The step-by-step guidance, screenshots, and links to GitHub's OAuth setup page and documentation significantly improve the user experience.
Consider the following minor improvement:
-For development, make sure you setup this with a **Homepage URL** of +For development, make sure you set up this with a **Homepage URL** ofAdditionally, consider adding a note about the security implications of handling the GitHub secret, emphasizing the importance of not committing it to version control.
🧰 Tools
🪛 LanguageTool
[typographical] ~94-~94: It seems that a comma is missing.
Context: ...ow authentication via GitHub. To enable this you need to have aGITHUB_IDand `GIT...(IN_ORDER_TO_VB_COMMA)
[grammar] ~95-~95: This sentence should probably be started with a verb instead of the noun ‘Setup’. If not, consider inserting a comma for better clarity.
Context: ...GITHUB_IDandGITHUB_SECRETvalue. Setup your GitHub ID & Secret on GitHub: - [C...(SENT_START_NN_DT)
[grammar] ~100-~100: The word “setup” is a noun. The verb is spelled with a space.
Context: ...below. For development, make sure you setup this with a Homepage URL of ``` htt...(NOUN_VERB_CONFUSION)
🪛 Markdownlint
101-101: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
105-105: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- README.md (3 hunks)
🧰 Additional context used
🪛 LanguageTool
README.md
[grammar] ~47-~47: This sentence should probably be started with a verb instead of the noun ‘Setup’. If not, consider inserting a comma for better clarity.
Context: ...tabase is running](#database_url). 6. Setup the tables in the database with Drizzle...(SENT_START_NN_DT)
[grammar] ~100-~100: The word “setup” is a noun. The verb is spelled with a space.
Context: ...below. For development, make sure you setup this with a Homepage URL of ``` htt...(NOUN_VERB_CONFUSION)
🪛 Markdownlint
README.md
101-101: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
105-105: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
🔇 Additional comments (3)
README.md (3)
60-60: Approve addition of package.json link.The added link to the package.json file for the full db:seed command is consistent with the previous link for the db:push command. This provides transparency and allows users to easily find the full command details.
117-119: Approve addition of GitHub OAuth documentation link.The added link to GitHub's documentation on Authorizing OAuth apps is a valuable resource for users who want to dive deeper into the OAuth setup process. This addition enhances the overall quality of the documentation.
Line range hint
1-190: Overall assessment: Significant improvement to documentationThe changes to the README.md file represent a substantial improvement to the project documentation. Key enhancements include:
- Clearer instructions for database setup and seeding.
- Detailed guidance for GitHub OAuth configuration, including screenshots and links to relevant resources.
- Improved transparency with links to the package.json file for command details.
These additions will greatly assist new contributors in setting up the project and understanding its configuration requirements. The step-by-step approach and visual aids make the documentation more user-friendly and accessible.
A few minor grammatical issues were noted and can be easily addressed. Consider implementing the suggested changes to further polish the documentation.
Great work on enhancing the project's onboarding experience!
🧰 Tools
🪛 LanguageTool
[grammar] ~47-~47: This sentence should probably be started with a verb instead of the noun ‘Setup’. If not, consider inserting a comma for better clarity.
Context: ...tabase is running](#database_url). 6. Setup the tables in the database with Drizzle...(SENT_START_NN_DT)
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
NiallJoeMaher
changed the title
3 participants
✨ Codu Pull Request 💻
Fixes #(issue)
Pull Request details
Any Breaking changes
Associated Screenshots