Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Print shorter versions of git commit hashes in `dvc diff` #1902

Closed
jorgeorpinel opened this issue Apr 18, 2019 · 6 comments

Comments

Projects
None yet
3 participants
@jorgeorpinel
Copy link
Contributor

commented Apr 18, 2019

Per iterative/dvc.org#244 (comment), requested by @shcheklein

Currently dvc diff outputs a line such as

dvc diff from 6a819f8fa053f124f6a5487efc824a8c17366c71 to 612f6caf5c5daeb172167db285efd8b169d41b60

with the full-length commit hashes. Can we just print the first 7 characters like Git and GitHub support?

@jorgeorpinel

This comment has been minimized.

Copy link
Contributor Author

commented Apr 18, 2019

Ping @efiop what do you think? If you approve, I can try to implement this myself and send a PR. Any tips on what source code to focus on will be appreciated if so.

@efiop

This comment has been minimized.

Copy link
Member

commented Apr 18, 2019

@jorgeorpinel Great idea! All diff-related logic is in dvc/repo/diff.py dvc/command/diff.py and dvc/scm/git/__init__.py pretty much. The part that needs adjusting is most likely in dvc/scm/git/__init__.py::Git::get_diff_trees. Thanks for looking into it 🙂 Let us know if you need any additional help with this patch.

@shcheklein

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

@jorgeorpinel thanks! 👍 do you want to assign it to yourself?

@jorgeorpinel

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

Thanks @efiop. I do need some more guidance. Questions:

  • Most importantly: Is there a README on how to install the dvc repo locally for development? I guess just pip install -r requirements.txt but when how do I run that local copy of DVC? To test my changes.
  • Less relevant but I'm curious about: dvc/scm/git/__init__.py::Git::_get_diff_trees has
    from gitdb.exc import BadObject, BadName
    (which seems to be the only place where gitdb is used in the entire repo?)
    but a) I can't find gitdb in requirements.txt and b) I can't find gitdb.exc in the gitdb API ref. Is the exc module maybe from an older version of gitdb? I'm a bit lost with that one, sorry.
@shcheklein

This comment has been minimized.

Copy link
Member

commented Apr 19, 2019

@jorgeorpinel I think this a good starting point to setup dev environment - https://dvc.org/doc/user-guide/contributing

https://github.com/gitpython-developers/GitPython pulls gitdb with it. I'm not sure about the version though.

@jorgeorpinel

This comment has been minimized.

Copy link
Contributor Author

commented Apr 19, 2019

Thanks Ivan!

I followed the contributing guide which was quite straightforward, thanks. This allowed me to test my small change locally and it seems it works. Here's the PR: #1906

Regarding gitdb I do see it in the reqs of gitpython now, thanks. And it is a pretty outdated version it seems (0.6.4 while the latest is 2.0.5 atm) But still I can't find the exc module in that old version's API ref. Oh well, don't really need that.

jorgeorpinel added a commit to jorgeorpinel/dvc that referenced this issue Apr 25, 2019

diff: Changes to dvc.scm.git so `dvc diff` outputs short SHAs
- Docstring for Git class constructor
- Renames self.git for self.repo throughout module (involves several test changes)
- Better doc and logical refactor in Git._get_diff_trees
- `dvc diff` now outputs short SHA strings (Git._get_diff_trees) - hard coded to 7 length str
  (involves some test changes in tests/func/test_diff.py)

For iterative#1902

@efiop efiop closed this in #1906 May 10, 2019

efiop added a commit that referenced this issue May 10, 2019

Document dvc.scm.git::Git::_get_diff_trees and shorten commit hashes …
…for `dvc diff` (#1906)

* Document dvc.scm.git::Git::_get_diff_trees and shorten commit hashes in dvc.scm.git::Git::get_diff_trees

* diff: Changes to dvc.scm.git so `dvc diff` outputs short SHAs

- Docstring for Git class constructor
- Renames self.git for self.repo throughout module (involves several test changes)
- Better doc and logical refactor in Git._get_diff_trees
- `dvc diff` now outputs short SHA strings (Git._get_diff_trees) - hard coded to 7 length str
  (involves some test changes in tests/func/test_diff.py)

For #1902

* Undoes "Renames self.git for self.repo throughout module" introduced in 3d2dc83.

* diff: `dvc diff` uses code for `git rev_parse --short` instead of hard `[:7]` (for short commit hashes).

- Tests updated too.

Per #1906 (comment)

* tests: Reformatted with `black tests/func/test_diff.py`

* tests: Remove uneccessary `str()` casting form `dvc diff` tests (and reformat)

jorgeorpinel added a commit to jorgeorpinel/dvc that referenced this issue May 16, 2019

diff: Changes to dvc.scm.git so `dvc diff` outputs short SHAs
- Docstring for Git class constructor
- Renames self.git for self.repo throughout module (involves several test changes)
- Better doc and logical refactor in Git._get_diff_trees
- `dvc diff` now outputs short SHA strings (Git._get_diff_trees) - hard coded to 7 length str
  (involves some test changes in tests/func/test_diff.py)

For iterative#1902
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.