-
Notifications
You must be signed in to change notification settings - Fork 150
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
docs: add well documented with usage example for the restoreTimeStr #7045
Conversation
pls doc the API |
I guess doc the api |
Signed-off-by: maskedemann <[email protected]>
Hi @maskedeman, thanks for your contribution. The API reference doc is generated from the source code comment, so it's not necessary to update the API reference doc manually. Better to update the comment of
And run |
Signed-off-by: maskedemann <[email protected]>
//The function follows a specific time format/layout constraint for the restoreTimeStr API, | ||
//which is "Jan 02, 2006 15:04:05 UTC-0700". The restoreTimeStr is formatted using the RFC3339 format, after parsing this string into a time.Time object. | ||
//Example usage: | ||
// Define a restore time string | ||
// restoreTimeStr := "Jan 02,2006 15:04:05 UTC-0700" | ||
// Use the function | ||
// formattedTime, err := FormatRestoreTimeAndValidate(restoreTimeStr, backup) | ||
// if err != nil { | ||
// fmt.Println("Error:", err) | ||
// } | ||
// fmt.Println("Formatted time:", formattedTime) | ||
// Parameters: | ||
// - restoreTimeStr: A string representing a time in the given format. | ||
// - backup: A pointer to the backup object that contains the time range | ||
// for validation. | ||
// Returns: | ||
// - string: The formatted restore time string. | ||
// - error: An error if the restore time string cannot be parsed or is outside the time range. |
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.
This is the comment of FormatRestoreTimeAndValidate
, these comments are not suitable as the description for this field.
I suggest the following would be sufficient, just add allowed format description.
// Defines the point in time to restore. The allowed time formats are as follows:
// - `Jan 02, 2006 15:04:05 UTC-0700`
// - RFC3339 format, such as `2023-05-06T20:04:05+08:00`
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.
And the latest main branch code already added the format description:
// Specifies the point in time to which the restore should be performed.
// Supported time formats:
//
// - RFC3339 format, e.g. "2023-11-25T18:52:53Z"
// - A human-readable date-time format, e.g. "Jul 25,2023 18:52:53 UTC+0800"
//
Returns: | ||
- string: The formatted restore time string. | ||
- error: An error if the restore time string cannot be parsed or is outside the time range. | ||
*/ |
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.
To be honest, I think the comments on this function are not very necessary. The function is simple enough and the logic is already very clear. There is no need to explain the execution logic of the function again in the comments. If really want to add a comment, I think one sentence is enough.
// FormatRestoreTimeAndValidate formats the restore time and validates if it is in
// the time range of the continuous backup.
/* | ||
isTimeInRange checks if the given time is within the specified time range. | ||
It returns true if the time is within the range, otherwise 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.
This comment is also somewhat redundant.
func FormatRestoreTimeAndValidate(restoreTimeStr string, continuousBackup *dpv1alpha1.Backup) (string, error) { | ||
if restoreTimeStr == "" { | ||
return restoreTimeStr, nil | ||
} | ||
// layout defines the time format/layout constraint for the restoreTimeStr API. | ||
// The layout follows the reference time "Jan 02, 2006 15:04:05 UTC-0700". | ||
// It specifies the expected format of the restore time string. |
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.
Reserve // layout defines the time format/layout constraint for the restoreTimeStr API
is enough.
opsrequest.spec.restoreSpec.restoreTimeStr
#7031