Skip to content

Fix rename change type & support 'change in type' #755

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 2 commits into from
May 19, 2018

Conversation

LeResKP
Copy link
Contributor

@LeResKP LeResKP commented May 16, 2018

Hello,

It fixes #563, we now parse correctly the change type and store the score if given. I didn't manage to create a copied status in my git so I didn't support it for now. Hope I will find some time later to give another try.

Regards,
Aurélien

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 your contribution. Something stuck out that I think needs fixing.
After that I am happy to merge.

git/diff.py Outdated
# Change type can be R100
# R: status letter
# 100: score (in case of copy and rename)
change_type = change_type[0]
Copy link
Member

Choose a reason for hiding this comment

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

This one is interesting: change_type is reassigned, and thus just a single character. One line later, change_type is treated as the original string to get the score, which probably is always empty now.

self.assertEqual(diff.rename_from, None)
self.assertEqual(diff.rename_to, None)
self.assertEqual(diff.change_type, 'T')
self.assertEqual(len(list(diffs.iter_change_type('T'))), 1)
Copy link
Member

Choose a reason for hiding this comment

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

I think a test for score is missing. It's likely to not work in the current implementation.

@Byron Byron added this to the v2.1.10 - Bugfixes milestone May 18, 2018
@Byron Byron mentioned this pull request May 18, 2018
@codecov-io
Copy link

codecov-io commented May 18, 2018

Codecov Report

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

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #755      +/-   ##
==========================================
+ Coverage   94.67%   94.68%   +<.01%     
==========================================
  Files          59       59              
  Lines        9322     9339      +17     
==========================================
+ Hits         8826     8843      +17     
  Misses        496      496
Impacted Files Coverage Δ
git/diff.py 99.13% <100%> (+0.01%) ⬆️
git/test/test_diff.py 100% <100%> (ø) ⬆️
git/test/test_repo.py 97.83% <0%> (-0.05%) ⬇️
git/cmd.py 83.62% <0%> (ø) ⬆️

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 7be3486...1c27814. Read the comment docs.

@LeResKP
Copy link
Contributor Author

LeResKP commented May 18, 2018

oh sorry, I didn't red this old commit correctly, it was not finished.... I fixed it.

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 so much for the quick fix and your contribution.
I will cut a new release now.

@Byron Byron merged commit 29aa1b8 into gitpython-developers:master May 19, 2018
@Thrimbda
Copy link

glad to see this issue solved

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

Successfully merging this pull request may close these issues.

rename file got diff with change_type: R100
4 participants