Skip to content

git: index: base: use os.path.relpath #744

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

Merged
merged 1 commit into from
Apr 4, 2018

Conversation

efiop
Copy link
Contributor

@efiop efiop commented Apr 3, 2018

Fixes #743

Signed-off-by: Ruslan Kuprieiev [email protected]

@efiop efiop force-pushed the master branch 5 times, most recently from 97164b0 to 176474b Compare April 3, 2018 13:30
@codecov-io
Copy link

codecov-io commented Apr 3, 2018

Codecov Report

Merging #744 into master will increase coverage by <.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #744      +/-   ##
==========================================
+ Coverage   94.66%   94.66%   +<.01%     
==========================================
  Files          59       59              
  Lines        9289     9298       +9     
==========================================
+ Hits         8793     8802       +9     
  Misses        496      496
Impacted Files Coverage Δ
git/index/base.py 94.63% <100%> (-0.02%) ⬇️
git/test/test_index.py 96.49% <100%> (+0.06%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e79a3f8...c554ab1. Read the comment docs.

Copy link
Member

@Byron Byron left a comment

Choose a reason for hiding this comment

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

Thanks a lot for submitting the fix! There is only a minor issue preventing a merge.
Thanks for having a look.

repo = Mocked()
path = os.path.join(root, 'file')
index = IndexFile(repo)
index._to_relative_path(path)
Copy link
Member

Choose a reason for hiding this comment

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

Is there an assertion missing? It looks like _to_relative_path returns the relative path, which is not verified here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, right. Thank you, fixed in a new version.

@Byron Byron added this to the v2.1.10 - Bugfixes milestone Apr 3, 2018
@Byron Byron merged commit 0857d33 into gitpython-developers:master Apr 4, 2018
@Byron
Copy link
Member

Byron commented Apr 4, 2018

Thanks a lot! Looking good!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants