Skip to content

Conversation

@LeResKP
Copy link
Contributor

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

@ByronByron 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.

@ByronByron added this to the v2.1.10 - Bugfixes milestone May 18, 2018
@ByronByron 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 FilesCoverage Δ
git/diff.py99.13% <100%> (+0.01%)⬆️
git/test/test_diff.py100% <100%> (ø)⬆️
git/test/test_repo.py97.83% <0%> (-0.05%)⬇️
git/cmd.py83.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
ContributorAuthor

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

Copy link
Member

@ByronByron 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.

@ByronByron merged commit 29aa1b8 into gitpython-developers:masterMay 19, 2018
@Thrimbda
Copy link

glad to see this issue solved

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

Labels

Development

Successfully merging this pull request may close these issues.

rename file got diff with change_type: R100

4 participants

@LeResKP@codecov-io@Thrimbda@Byron