Skip to content

Fix bug in moving files between subfolders. #105

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.

Sign up for GitHub

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

Jump to bottom
Merged
diwakergupta merged 3 commits into from
Dec 5, 2018
Merged

Fix bug in moving files between subfolders. #105

diwakergupta merged 3 commits into from
Dec 5, 2018

Conversation

@vaithak
Copy link
Contributor

@vaithak vaithak commented Oct 19, 2018 •
edited
Loading

Fixes #102 , Fixes #104

@vaithak
Copy link
Contributor Author

vaithak commented Oct 19, 2018

This PR fixes two issues - #102 & #104 . Please review this.
Thanks.

@diwakergupta
Copy link
Collaborator

diwakergupta commented Oct 20, 2018

@vaithak thanks. Can you split this up into separate PRs for each issue? For #102 I'd also recommend adding something to contrib/test.sh that validates the behavior. We don't have comprehensive integration tests, but some way to verify before / after would help.

@vaithak
Copy link
Contributor Author

vaithak commented Oct 20, 2018 •
edited
Loading

@diwakergupta. I have split the pull requests, now this PR fixes #102. There is already a test in contrib/test.sh for mv command.
Can you please elaborate what else test should I write in it ?

@vaithak vaithak changed the title Fix bug in moving files between subfolders and add feature to add keys using environment variables. Fix bug in moving files between subfolders. Oct 20, 2018
@diwakergupta
Copy link
Collaborator

diwakergupta commented Oct 20, 2018

@vaithak well does the current mv usage in test.sh fail without this fix? If not, could you add something that fails without your patch and succeeds with it?

@vaithak
Copy link
Contributor Author

vaithak commented Oct 21, 2018

@diwakergupta I have added tests as well as updated mv.go .

@vaithak
Copy link
Contributor Author

vaithak commented Oct 25, 2018

@diwakergupta Please review my PR.

diwakergupta
diwakergupta requested changes Oct 25, 2018
View reviewed changes
@vaithak
Copy link
Contributor Author

vaithak commented Oct 26, 2018

@diwakergupta I have added the requested changes. Please review them.

@vaithak
Copy link
Contributor Author

vaithak commented Nov 8, 2018

@diwakergupta I have made all the changes, please accept my PR.

@diwakergupta diwakergupta merged commit f0a17d9 into dropbox:master Dec 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@diwakergupta diwakergupta diwakergupta requested changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@vaithak @diwakergupta