Skip to content

dvcignore: api to unhide subrepos directory#4324

Merged
efiop merged 2 commits into
treeverse:masterfrom
skshetry:track-subrepos-dvcignore
Aug 10, 2020
Merged

dvcignore: api to unhide subrepos directory#4324
efiop merged 2 commits into
treeverse:masterfrom
skshetry:track-subrepos-dvcignore

Conversation

@skshetry
Copy link
Copy Markdown
Collaborator

@skshetry skshetry commented Aug 3, 2020

Thank you for the contribution - we'll try to review it as soon as possible. 🙏

Part of #3369

I implemented this inside dvcignore, and it
should not require "internal" APIs hack as we discussed before.

Right now, ignore_subrepos will only work for the paths of subrepo,
not anything inside it. And the API user is more or less only
RepoTree, so should be safe (we switch DvcTree and repo.tree based
on path prefixes, so this is only required during walk, so as it
shows the repo that were previously dvcignored and so that we could
switch the trees later during walk.

So, if dir is dir -> repo
So, tree.walk("dir") will return [],
whereas tree.walk("dir", ignore_subrepos=False) will return ["repo"]
But, successive walk will just return ("repo", [], []), and could
just be ignored as a side-effect.

@skshetry skshetry requested a review from efiop August 3, 2020 13:14
@skshetry skshetry self-assigned this Aug 3, 2020
@skshetry skshetry marked this pull request as draft August 3, 2020 13:39
Comment thread dvc/ignore.py Outdated
Comment thread dvc/ignore.py Outdated
Comment thread dvc/ignore.py Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I know that fstrings are better than format() , but there wasn't really a reason to change it in this PR 🙂

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I had to change that before when I was experimenting with it. And, f-string is easy to read and understand (here, speed does not matter but could have been another reason), and therefore, I kept it.

Comment thread dvc/ignore.py Outdated
Comment thread dvc/ignore.py Outdated
I implemented this inside dvcignore, and it
should not require "internal" APIs hack as we discussed before.

Right now, `ignore_subrepos` will only work for the paths of subrepo,
not anything inside it. And the API user is more or less only
RepoTree, so should be safe (we switch DvcTree and repo.tree based
on path prefixes, so this is only required during `walk`, so as it
shows the repo that were previously dvcignored and so that we could
switch the trees later during walk.

So, if dir is `dir -> repo`
So, `tree.walk("dir")` will return `[]`,
whereas `tree.walk("dir", ignore_subrepos=False)` will return `["repo"]`
But, successive walk will just return `("repo", [], [])`, and could
just be ignored as a side-effect.
@skshetry skshetry force-pushed the track-subrepos-dvcignore branch from 4e013e1 to bc01116 Compare August 6, 2020 14:58
@skshetry skshetry marked this pull request as ready for review August 6, 2020 15:03
@skshetry skshetry changed the title Track ignored subrepos by dvcignore dvcignore: api to unhide subrepos directory Aug 6, 2020
@efiop efiop merged commit 8d1934d into treeverse:master Aug 10, 2020
@skshetry skshetry deleted the track-subrepos-dvcignore branch August 10, 2020 07:00
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.

2 participants