-
Notifications
You must be signed in to change notification settings - Fork 88
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
Split device sync into readable functions #879
Split device sync into readable functions #879
Conversation
Signed-off-by: Bala.FA <bala@minio.io>
c8f0f2c
to
93783b6
Compare
drive.Spec.Relabel = false | ||
} | ||
|
||
return true |
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.
shouldn't we return the updated
var instead? and we can set this var to true whenever the drive fields are updated. There are cases where we don't need the drive update.
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.
In error case and success case, we should set updated = true
. This means we are returning true
in any case. That's why return true
is correct.
} | ||
|
||
if err == nil { | ||
drive.Spec.Relabel = false |
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.
can we set updated = true
here and return at the end?
default: | ||
if latestErrorConditionType != directpvtypes.DriveConditionTypeIOError { | ||
drive.Status.Status = directpvtypes.DriveStatusError | ||
drive.SetRelabelErrorCondition(fmt.Sprintf("unable to relabel; %v", err)) |
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.
can we set updated = true
here and return at the end?
No description provided.