-
Notifications
You must be signed in to change notification settings - Fork 60
nvme: Fix typo in BD_NVME_SANITIZE_STATUS_IN_PROG*R*ESS #1153
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
nvme: Fix typo in BD_NVME_SANITIZE_STATUS_IN_PROG*R*ESS #1153
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded a canonical enum member Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (690)
🔇 Additional comments (3)
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 |
Summary of ChangesHello @vojtechtrefny, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses a minor but important issue by correcting a typo in the documentation for an NVMe sanitize status enum. To prevent breaking existing code that might rely on the original misspelled enum member, a new, correctly spelled alias has been introduced. This approach ensures that the documentation is accurate while maintaining full backward compatibility for the API. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request aims to fix a typo in the BD_NVME_SANITIZE_STATUS_IN_PROGESS enum member. While the intent to maintain backward compatibility by keeping the old member is good, the implementation has a couple of issues. The new enum member is named BD_NVME_SANITIZE_STATUS_PROGRESS instead of the likely intended BD_NVME_SANITIZE_STATUS_IN_PROGRESS. Additionally, the documentation update replaces the entry for the old member, which could be confusing. I've provided suggestions to correct the name of the new enum member and to adjust the documentation to be clearer by adding a new entry for the new member and marking the old one as deprecated.
33afa01 to
353225f
Compare
tbzatek
left a 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.
I would perhaps do it the other way around - have the new name as =1 primarily and the assignment with the typo as a compat fallback...
353225f to
8eaf8e4
Compare
Unfortunately we need to keep the "progess" member as well now.
8eaf8e4 to
0e14f82
Compare
Makes sense, this way, it will be simple to remove it when we release libblockdev 4.0 in 10 years :-) |
|
Funnily enough, we only copied the typo from libnvme linux-nvme/libnvme#1094 :-) |
tbzatek
left a 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.
Thanks! Would you like to fix UDisks as well? :-p
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
Unfortunately we need to keep the "progess" member as well now.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.