Skip to content
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 non-directory source path handling #71

Merged
merged 7 commits into from Oct 1, 2020
Merged

Fix non-directory source path handling #71

merged 7 commits into from Oct 1, 2020

Conversation

at-wat
Copy link
Member

@at-wat at-wat commented Sep 30, 2020

Fix #69

@at-wat at-wat self-assigned this Sep 30, 2020
@codecov
Copy link

codecov bot commented Sep 30, 2020

Codecov Report

Merging #71 into master will increase coverage by 3.22%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #71      +/-   ##
==========================================
+ Coverage   76.25%   79.47%   +3.22%     
==========================================
  Files           4        4              
  Lines         240      268      +28     
==========================================
+ Hits          183      213      +30     
+ Misses         32       31       -1     
+ Partials       25       24       -1     
Impacted Files Coverage Δ
s3sync.go 77.27% <100.00%> (+3.90%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 268cc6e...033eb29. Read the comment docs.

@at-wat at-wat marked this pull request as ready for review October 1, 2020 02:47
@at-wat at-wat requested review from kt3k and kamatama41 October 1, 2020 02:47
@at-wat
Copy link
Member Author

at-wat commented Oct 1, 2020

@kt3k @kamatama41 please take a look

Comment on lines +381 to +389
if name == "." {
// Single file was specified
c <- &fileInfo{
name: filepath.Base(*object.Key),
path: filepath.Dir(*object.Key),
size: *object.Size,
lastModified: *object.LastModified,
singleFile: true,
}
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

s3sync_test.go Outdated
if err := New(getSession()).Sync("s3://example-bucket/README.md", filepath.Join(temp, "foo")+"/"); err != nil {
t.Fatal("Sync should be successful", err)
}
// Download to ./foo/README.md
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe this should be Download to ./test.md?

Copy link
Contributor

@kt3k kt3k left a comment

Choose a reason for hiding this comment

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

LGTM with nits

@kt3k kt3k merged commit 490561e into master Oct 1, 2020
@kt3k kt3k deleted the fix-non-dir-source branch October 1, 2020 08:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Source local path with filename is not handled properly
2 participants