casperdcl / git-fame

:star: Pretty-print `git` repository collaborators sorted by contributions
Other
641 stars 34 forks source link

wrong loc after author change or repo import #85

Closed KrisThielemans closed 1 year ago

KrisThielemans commented 1 year ago

A while ago, I needed to incorporate someone else's code into our STIR repo. Unfortunately, I had commited his code in the original repo. So I manipulated history to assign some commits to him. (Sadly, I didn't record exactly what I did but I followed roughly https://stackoverflow.com/a/28845565/15030207). I then incorporated the repo into our STIR repo and moved files. (Roughly along the lines of https://stackoverflow.com/a/1684694/15030207). Checking now output of git fame, the relevant author doesn't get the credit he (=Carles Falcon) deserves. Am I doing something wrong with the call to git fame?

As running git fame -wMC on STIR take a very long time, I've tried to show an example with one of the relevant files is https://github.com/UCL/STIR/blob/master/src/recon_buildblock/PinholeSPECTUB_Weight3d.cxx, original name was wm_SPECT_mph2/weight3d_SPECT_mph.cpp`. For that I get

$ git fame  -wMC '--incl=PinholeSPECTUB_Weight3d.cxx|weight3d_SPECT_mph.cpp'
Total commits: 8021
Total ctimes: 30
Total files: 2
Total loc: 73
| Author                 |   loc |   coms |   fils |  distribution   |
|:-----------------------|------:|-------:|-------:|:----------------|
| Matthew Strugari       |    69 |     15 |      1 | 94.5/ 0.2/50.0  |
| Kris Thielemans        |     4 |   5625 |      1 | 5.5/70.1/50.0   |
| Alaleh Rashidnasab     |     0 |      1 |      0 | 0.0/ 0.0/ 0.0   |
| Alexander C. Whitehead |     0 |      5 |      0 | 0.0/ 0.1/ 0.0   |
| Alexey Zverovich       |     0 |      6 |      0 | 0.0/ 0.1/ 0.0   |
| Ander Biguri           |     0 |     63 |      0 | 0.0/ 0.8/ 0.0   |
| Ashley Gillman         |     0 |     50 |      0 | 0.0/ 0.6/ 0.0   |
| Benjamin Thomas        |     0 |     22 |      0 | 0.0/ 0.3/ 0.0   |
| C. Ross Schmidtlein    |     0 |      2 |      0 | 0.0/ 0.0/ 0.0   |
| Carles Falcon          |     0 |      2 |      0 | 0.0/ 0.0/ 0.0   |
...

Note that the "Total loc" reported is 73, while actually the file has some 1100 lines. All the ones from Carles are excluded for some reason. Of course, I could be making a mistake with the --incl option but Carles gets no credit when I don't specify an include.

git blame reports the correct thing, see here. Carles gets the correct number of commits, so maybe it's the way that I merged the original repo into STIR? (e.g. the commit does not "follow" from the first STIR commit).

The "re-authored" commit is https://github.com/UCL/STIR/commit/dd6fdee0aaa5871c75cf89e239bbf464110b3803. The PR with the move (and other fixes) is https://github.com/UCL/STIR/pull/1100

KrisThielemans commented 1 year ago

Something else that I notice is that the number of commits reports is not to the files included, but the whole repo. That might be intentional though.

KrisThielemans commented 1 year ago

A simpler case is probably https://github.com/UCL/STIR/blame/master/src/include/stir/NumericType.h where I didn't do the author change. The initial revision was by Alexei Zverovich, https://github.com/UCL/STIR/commit/691d037720d963849ed9d2e2cb447814e88ed652, and some of his lines survive, (Weirdly, he's listed as a committer in the output above, but not when I don't have the --incl line). . This goes back 23 years though, and I remember having to do some filter-branch stuff to remove non-open source files, so again the history isn't quite standard.

(Edited: I've now re-run git fame -wMC on STIR without the --since and Alexei is credited ok, so forget about the "simpler example". sorry for the confusion!)

casperdcl commented 1 year ago

Sorry for the delay; thanks for reporting this. Should be fixed in git-fame>=2.0.1