Skip to content
This repository has been archived by the owner on May 4, 2021. It is now read-only.

Refactor copy related logic #320

Merged
merged 3 commits into from
Apr 23, 2020
Merged

Refactor copy related logic #320

merged 3 commits into from
Apr 23, 2020

Conversation

yiranwang52
Copy link
Contributor

No description provided.

@@ -272,7 +280,7 @@ func (c copier) copyDirContents(src, dst, origDst string, uid, gid int, preserve
}

// copyDir copies the directory at src to dst.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it might be better to maintain the original naming. The current naming seems to copy the child of src but actually src itself is the child.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

blacklist []string

dstDirOwner *Owner
dstFileAndChildrenOwner *Owner
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you clarify what dstFileAndChildrenOwner means?

)

// fileOwners returns the uid & gid that own the file.
func fileOwners(fi os.FileInfo) (uid int, gid int) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

may be more intuitive to call it getFileOwners.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

for _, dir := range split {
absDir := filepath.Join(prevDir, dir)
if fi, err := os.Lstat(absDir); err != nil {
for i, dir := range split {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for dir in range split[:len(split) -1]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

Copy link
Contributor

@jockech jockech left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work! Just leave few comments.

@yiranwang52 yiranwang52 merged commit d606f27 into master Apr 23, 2020
@yiranwang52 yiranwang52 deleted the yiran/bump branch April 23, 2020 17:14
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants