-
Notifications
You must be signed in to change notification settings - Fork 469
Invert password verification condition in Best Practice page #770
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: main
Are you sure you want to change the base?
Conversation
Add '!' in front of password verify condition make it's logic right
WalkthroughA logic error in an authentication example within documentation is corrected. The condition previously reversed the intended behavior, throwing an error on successful password verification instead of on failure. The conditional guard now correctly enforces authentication flow, allowing successful verification to proceed. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ 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 |
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
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/essential/best-practice.md
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
docs/essential/best-practice.md
98-98: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🔇 Additional comments (1)
docs/essential/best-practice.md (1)
98-103: ✅ Logic fix correctly implements authentication guard.The negation operator inverts the condition as intended: the error is now thrown on failed verification (false) rather than on successful verification (true). This aligns with standard authentication patterns and the example's purpose.
| LIMIT 1` | ||
|
|
||
| if (await Bun.password.verify(password, user.password)) | ||
| if (!await Bun.password.verify(password, user.password)) |
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.
Fix hard tab indentation; use spaces instead.
The indentation on this line uses hard tabs, which violates the Markdown linting rule MD010. Replace with spaces for consistency with Markdown best practices.
🔎 Proposed fix: Replace hard tab with spaces
- if (!await Bun.password.verify(password, user.password))
+ if (!await Bun.password.verify(password, user.password))(Replace the leading hard tab with two levels of space indentation to match the surrounding code block formatting.)
📝 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.
| if (!await Bun.password.verify(password, user.password)) | |
| if (!await Bun.password.verify(password, user.password)) |
🧰 Tools
🪛 markdownlint-cli2 (0.18.1)
98-98: Hard tabs
Column: 1
(MD010, no-hard-tabs)
🤖 Prompt for AI Agents
In docs/essential/best-practice.md around line 98, the line "if (!await
Bun.password.verify(password, user.password))" is indented with a hard tab which
violates MD010; replace the leading hard tab with spaces (use two-level space
indentation to match the surrounding fenced code block) so the line uses only
spaces for indentation and aligns with neighboring lines.
The original condition throw 400 error when the password is verified,
thereforce add negate symbol in front of password verify condition negate the result make it right
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.