During work on #163, I noticed that the InvocationCounter in the Analysis which counts all commits in the repository (even filtered ones) is never used. This is due to 36dc1bb which left that artifact behind.
However, the semantics changed slightly:
Previously, it counted the commits before the DiffFilter.
Now, it only counts commits after the DiffFilter.
We should decide which behavior is actually intended and document it as such.
Note that the documentation contains some improvement potential in this regard:
- Which behavior does this describe?
|
* {@code filteredCommits = totalCommits - processedCommits - emptyCommits - failedCommits} |
- This formula contains a typo and implies that
FilterAnalysis filters commits but it only filters VariationDiffs.
|
* {@link Result#emptyCommits} + {@link Result#failedCommits} + {@link Result#processedCommits} - {@link FilterAnalysis filteredCommits} = {@link Analysis.TotalNumberOfCommitsResult#value} |
Furthermore, there is a bug in Analysis.forSingleCommit: It increases the total commits although this is already done in processCommit. So the result will have a total commit count of 2. (Note that I didn't test this. This is just my mental model of the code.)
Regarding FilterAnalysis: This could in principle replace DiffFilter as implied by the comment in FilterAnalysis. For commits, this would already work. For patches, we would need to introduce a new hook, something like beginParsePatch (because commits are fully parsed before beginPatch is ever called), and do a bunch of refactorings.
During work on #163, I noticed that the
InvocationCounterin theAnalysiswhich counts all commits in the repository (even filtered ones) is never used. This is due to 36dc1bb which left that artifact behind.However, the semantics changed slightly:
Previously, it counted the commits before the
DiffFilter.Now, it only counts commits after the
DiffFilter.We should decide which behavior is actually intended and document it as such.
Note that the documentation contains some improvement potential in this regard:
DiffDetective/src/main/java/org/variantsync/diffdetective/analysis/StatisticsAnalysis.java
Line 41 in 19a8518
FilterAnalysisfilters commits but it only filtersVariationDiffs.DiffDetective/src/main/java/org/variantsync/diffdetective/analysis/StatisticsAnalysis.java
Line 24 in 19a8518
Furthermore, there is a bug in
Analysis.forSingleCommit: It increases the total commits although this is already done inprocessCommit. So the result will have a total commit count of 2. (Note that I didn't test this. This is just my mental model of the code.)Regarding
FilterAnalysis: This could in principle replaceDiffFilteras implied by the comment inFilterAnalysis. For commits, this would already work. For patches, we would need to introduce a new hook, something likebeginParsePatch(because commits are fully parsed beforebeginPatchis ever called), and do a bunch of refactorings.