Dvc: rename `git` property (a Repo object) in class `dvc.scm.Git`?

Created on 12 May 2019  路  3Comments  路  Source: iterative/dvc

class Git(Base) property self.git, while named git, is not a Git object but rather a Repo object, which comes with a git property itself (which is the actual Git object); So using the latter (which we do a lot in tests) makes the code a little confusing, like: self.git.git and dvc.scm.git.git.

It could even be self.git.git.git in class GitTree!

I think ideally the property name should be self.repo, except that name its already in use by class Base (parent of class Git) so renaming dvc.scm.Git.git to dvc.scm.Git.repo would overwrite dvc.scm.Base.repo. I don't know whether this is an immediate problem since I don't see the current dvc.scm.Git.repo property being used anywhere in the code, but we probably want to avoid that overwrite anyway.

The options I see are:
a) Don't mind the overwrite and just rename dvc.scm.Git.git to dvc.scm.Git.repo
b) Rename dvc.scm.Base.repo to something else (e.g. dvc.scm.Base._repo) and then rename dvc.scm.Git.git to dvc.scm.Git.repo
c) Rename dvc.scm.Git.git to something else, e.g. dvc.scm.Git.repository

c1-quick-fix enhancement good first issue help wanted

All 3 comments

I'm going to vote for option b) here.

@jorgeorpinel Hm, I think you are right about scm.Base.repo not being used. scm should be standalone and should not depend on dvc Repo instance. It might be just a trace of some legacy change that we didn't take good enough care of when cleaning out. If that is the case and you can avoid passing repo to SCM at all, we can safely rename self.git -> self.repo.

I've taken this issue

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jorgeorpinel picture jorgeorpinel  路  45Comments

danfischetti picture danfischetti  路  41Comments

pared picture pared  路  73Comments

ynop picture ynop  路  41Comments

dmpetrov picture dmpetrov  路  35Comments