-
Notifications
You must be signed in to change notification settings - Fork 20
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
Fix SOTA to not reboot twice on EXT4 system #481
Conversation
RTC 530960 Fix SOTA snapshot conditionali statements
/Review |
Code Review Analysis
Code Review Feedback💡 General suggestions: The PR seems to address the issue described, and the changes are clear and concise. However, it is important to ensure that the snapshot number "0" is indeed the correct indicator for no snapshot taken and that there are no edge cases where "0" could be a valid snapshot number. Additionally, it would be beneficial to add relevant unit tests to cover these changes and ensure that the new logic behaves as expected in different scenarios. ✨ Usage tips:
|
RTC 530960 Fix SOTA snapshot conditionali statements
/review |
|
Preparing review... |
Incremental PR Review
Code Review Analysis
Code Review FeedbackGeneral suggestions: The PR successfully addresses the critical issue of avoiding unnecessary reboots during the SOTA process on EXT4 systems by fixing conditional checks. It's essential to ensure that such changes are thoroughly tested across different system configurations to prevent any unforeseen issues. Additionally, considering the importance of sustainability mentioned in the PR description, it would be beneficial to include inline comments explaining the rationale behind key changes for future maintainability. User guidelines:
|
Preparing code suggestions... |
|
RTC 530960
Fix SOTA snapshot conditionali statements
The PR review is to check for sustainability and correctness. Sustainability is actually more business critical as correctness is largely tested into the code over time. Its useful to keep in mind that SW often outlives the HW it was written for and engineers move from job to job so it is critical that code developed for Intel be supportable across many years. It is up to the submitter and reviewer to look at the code from a perspective of what if we have to debug this 3 years from now after the author is no longer available and defect databases have been lost. Yes, that happens all the time when we are working with time scales of more than 2 years. When reviewing your code it is important to look at it from this perspective.
Author Mandatory (to be filled by PR Author/Submitter)
PULL DESCRIPTION
Provide a 1-2 line brief overview of the changes submitted through the Pull Request...
Fix SOTA snapshot conditionali statements
REFERENCES
Reference URL for issue tracking (JIRA/HSD/Github): <URL to be filled>
https://rtc.intel.com/ccm0006001/web/projects/SSEA-%20IOTG%20PED%20Team#action=com.ibm.team.workitem.viewWorkItem&id=530960&oslc_config.context=https://rtc.intel.com/gc/configuration/4491
Note-1: Depending on complexity of code changes, use the suitable word for complexity: Low/Medium/High
Example: PR for Slim boot loader project with medium complexity can have the label as: ISDM_Medium
CODE MAINTAINABILITY
APPLICATION SPECIFIC
Maintainer Mandatory (to be filled by PR Reviewer/Approving Maintainer)
QUALITY CHECKS
CODE REVIEW IMPACT
Note P1/P2/P3/P4 denotes severity of defects found (Showstopper/High/Medium/Low) and xx denotes number of defects found
SECURITY CHECKS
Please check if your PR fulfills the following requirements:
Code must act as a teacher for future developers