Skip to content

Conversation

@efiop
Copy link
Contributor

@efiop efiop commented Jul 2, 2020

Fixes #4150

Depends on #4155

  • ❗ I have followed the Contributing to DVC checklist.

  • πŸ“– If this PR requires documentation updates, I have created a separate PR (or issue, at least) in dvc.org and linked it here.

  • ❌ I will check DeepSource, CodeClimate, and other sanity checks below. (We consider them recommendatory and don't expect everything to be addressed. Please fix things that actually improve code or fix bugs.)

Thank you for the contribution - we'll try to review it as soon as possible. πŸ™

Ruslan Kuprieiev added 2 commits July 2, 2020 22:30
This PR is just a showcase to see how we like it. I've converted a few
tests to it, but there will be much more to come.

```
assert (tmp_dir / "file") == "contents"
assert (tmp_dir / "dir").read_text() == {"subdir": {"file": "hello"}}
```

This approach is nicer because it allows us to use a declaratory
asserts instead of relying on fs files. The problem becomes much more
apparent once you get into remote workspaces, where we `trees_equal`
doesn't work.
@efiop efiop requested a review from pmrowla July 2, 2020 19:49
Copy link
Contributor

@pmrowla pmrowla left a comment

Choose a reason for hiding this comment

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

LGTM (assuming nothing comes up in the read_text for dirs discussion)

@efiop efiop merged commit 8dd264c into treeverse:master Jul 3, 2020
@efiop efiop deleted the fix-4150 branch July 3, 2020 13:17
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.

DVC status fails after using add -R

2 participants