Skip to content

Make datasets.mv do nothing if source and destination are the same#162

Merged
acroz merged 1 commit into
masterfrom
datasets-mv-fix
May 4, 2020
Merged

Make datasets.mv do nothing if source and destination are the same#162
acroz merged 1 commit into
masterfrom
datasets-mv-fix

Conversation

@liamcoatman

Copy link
Copy Markdown
Contributor

Make datasets.mv do nothing if source and destination are the same.

Because mv is implemented as cp then rm, the previous behaviour was to delete the file if the source and destination are the same. The new behaviour is to do nothing (like the Unix mv command).

@imrehg imrehg left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch!

@acroz

acroz commented May 1, 2020

Copy link
Copy Markdown
Contributor

I suggest we close this in favour of #163. Then, it's the backend's responsibility to get this behaviour right. If the backend implements this behaviour wrongly, we should fix it. Thoughts?

@liamcoatman

Copy link
Copy Markdown
Contributor Author

I suggest we close this in favour of #163. Then, it's the backend's responsibility to get this behaviour right. If the backend implements this behaviour wrongly, we should fix it. Thoughts?

Agreed.

@liamcoatman liamcoatman closed this May 1, 2020
@jkeelan

jkeelan commented May 2, 2020

Copy link
Copy Markdown

I disagree, I think we should push this, because it prevents an unexpected and potentially destructive operation. To be clear, #163 also suffers from this issue, so it would have to be fixed on the backend, which will mean a further delay. While solving it on the front end isn't ideal, it's better than nothing.

@acroz

acroz commented May 4, 2020

Copy link
Copy Markdown
Contributor

Fair enough - let's merge this and deal with the backend implementation separately.

@acroz acroz reopened this May 4, 2020
@acroz acroz self-requested a review May 4, 2020 12:27
@acroz acroz force-pushed the datasets-mv-fix branch from cec1cd9 to 659352e Compare May 4, 2020 12:29
@acroz acroz merged commit 848d459 into master May 4, 2020
@acroz acroz deleted the datasets-mv-fix branch May 4, 2020 13:17
@acroz

acroz commented May 4, 2020

Copy link
Copy Markdown
Contributor

Released with 0.26.2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants