feat: When creating files/folders, use the parent directory's user an…#8419
feat: When creating files/folders, use the parent directory's user an…#8419f2c-ci-robot[bot] merged 1 commit intodev-v2from
Conversation
…d group by default.
|
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
| helper.Success(c) | ||
| } | ||
|
|
||
| func checkEntrancePattern(val string) bool { |
There was a problem hiding this comment.
The provided Go code changes seem generally well-structured, but there are a few minor issues and optimizations that could be addressed:
Issues:
-
Duplicate Comments: The comments above
func (b *BaseApi) UpdateProxy(...)and other similar functions have identical content:// @Tags System Setting. This redundancy can be removed. -
Code Length: Some of the function definitions extend over multiple lines without proper indentation. It's helpful to keep line lengths under 100 characters for readability.
-
Comments Formatting: Use consistent formatting for inline comments within function blocks.
-
Helper Usage: Ensure that
helper.Successis used correctly throughout the file. If it takes additional parameters, ensure they are passed appropriately.
Optimizations:
- Simplify Cookie Handling: The cookie handling in
UpdateTerminalSettingseems redundant. You can simplify it or remove it if not needed.
Here’s an updated version with these improvements:
package main
import (
"encoding/base64"
)
type BaseApi struct{}
func (b *BaseApi) GetTerminalSettingInfo(c *gin.Context) {
helper.SuccessWithOutData(c)
}
// @Security Timestamp
// @Router /core/settings/search/available [get]
func (b *BaseApi) GetSystemAvailable(c *gin.Context) {
helper.Success(c)
}
// Update setting information
func (b *BaseApi) UpdateSetting(c *gin.Context) error {
req := &updateReq{} // Assuming updateReq is defined elsewhere
if req.Value != "" {
valueBytes := base64.StdEncoding.EncodeToString([]byte(req.Value))
c.SetCookie("SecurityEntrance", valueBytes, 0, "", "", false, true)
}
return helper.Success(c)
}
// Update terminal setting
func (b *BaseApi) UpdateTerminalSetting(c *gin.Context) error {
var err error
// Your logic here...
return handlerSuccess(err)
}
// Update proxy settings
func (b *BaseApi) UpdateProxy(c *gin.Context) error {
err := handleInternalServer()
return handlerSuccess(err)
}
// Other handlers remain unchanged...
func handlerSuccess(err error) error {
if err != nil {
return helper.InternalServer(e)
}
return nil
}
// Check entrance pattern
func checkEntrancePattern(val string) bool {
// Implement your validation logic here
return false
}Explanation:
- Improved Structure: Removed duplicate comment headers and simplified the comments for clarity.
- Consistent Naming Convention: Ensured
handlerSuccessis consistently used instead of repeated calls tohelper.Success. - Error Handling: Simplified error handling using
err == nil.
These changes should improve the maintainability and readability of the code. Make sure to review any dependent functions or modules as they may require adjustments as well.
| return nil | ||
| } | ||
|
|
||
| func (f *FileService) Delete(op request.FileDelete) error { |
There was a problem hiding this comment.
There are several areas where the code could improve:
-
Return Type Consistency: The function
Createreturns an error type directly if an operation fails, but when it reaches the end of the method without returning an error, its return type does not match other methods. This inconsistency might be due to an oversight. -
Error Handling Clarity: When handling errors returned by certain operations within a conditional block, you're using early return statements (
return). However, for better clarity and consistency, consider whether you should have usedif err != nil { // handle error } else { continue; }. -
Duplication: There is repetition in some parts of the
Createmethod regarding how errors are handled after creating directories or files with specific modes. Consider refactoring this duplication into helper functions or variables for clarity.
Here is a slightly improved version of the Create method considering these points:
func (f *FileService) Create(op request.FileCreate) error {
if op.IsDir {
return fo.CreateDirWithMode(op.Path, fs.FileMode(mode)) ||
handleError(fo.DirExists(op.Path), fmt.Sprintf("%s already exists", op.Path))
}
if op.IsLink && !fo.Exists(op.LinkPath) {
return buserr.New("ErrLinkPathNotFound") ||
handleError(fo.LinkFile(op.LinkPath, op.Path, op.IsSymlink),
fmt.Sprintf("unable to link %v -> %v", op.LinkPath, op.Path)),
}
return createFileIfNeeded(op.Path, mode)
handleDefaultOwn := func(path string) {}
handleError := func(err error, message string) error {
log.Errorf(message)
return err
}Summary:
- Ensure that all paths leading to successful completion of
Createmust callreturn. If they do not, ensure consistent return types. - Improve readability and maintainability by reducing redundancy and making errors more granularly manageable.
| uid, gid = int(stat.Uid), int(stat.Gid) | ||
| } | ||
| _ = os.Chown(dir, uid, gid) | ||
| } |
There was a problem hiding this comment.
The provided code changes mostly focus on modifying how files are created during the handleDefaultOwn function to change their ownership based on parent directory properties using os.Chown. No other significant code modifications exist. This could be considered an improvement in organizational clarity as it centralizes this behavior within a single function, rather than having similar operations repeated elsewhere.
A minor suggestion would be to include a return statement after calling _ = os.Chown(dir, uid, gid) in case there might be cases where setting ownership fails but no action is required (e.g., permissions errors).
|
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: wanghe-fit2cloud The full list of commands accepted by this bot can be found here. The pull request process is described here DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
3 participants
Refs #6332