Skip to content

Conversation

@pared
Copy link
Contributor

@pared pared commented May 8, 2019

  • Have you followed the guidelines in our
    Contributing document?

  • Does your PR affect documented changes or does it add new functionality
    that should be documented? If yes, have you created a PR for
    dvc.org documenting it or at
    least opened an issue for it? If so, please add a link to it.


Fixes #1588

@pared pared force-pushed the 1588 branch 6 times, most recently from 20c47b4 to 83dc301 Compare May 9, 2019 03:14
@pared pared changed the title [WIP] path info: init path info: init May 9, 2019
@pared pared changed the title path info: init path info: replace path_info dicts with PathInfo class May 9, 2019
@pared pared requested a review from efiop May 9, 2019 16:38
@pared pared force-pushed the 1588 branch 4 times, most recently from 018473c to d844804 Compare May 10, 2019 21:09
dvc/output/gs.py Outdated
Copy link
Contributor

Choose a reason for hiding this comment

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

For the record: discussed introducing PathInfo(scheme, ...) function to help use common logic where possible.

Copy link
Contributor

@efiop efiop May 11, 2019

Choose a reason for hiding this comment

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

Seems like it should be in __init__, right?

@pared pared force-pushed the 1588 branch 2 times, most recently from 64ba0bc to aa0876a Compare May 11, 2019 03:31
Copy link
Contributor

@efiop efiop left a comment

Choose a reason for hiding this comment

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

Looking good! Great start! We'll be able to work on top of this easily if we ever decide to go with moving some logic like remote.ospath to PathInfos. Thanks!

@efiop efiop merged commit 3eaa725 into treeverse:master May 11, 2019
@pared pared deleted the 1588 branch November 12, 2019 08:54
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.

Refactoring: replace data value path_info by classes

2 participants