SLCORE-1142 Improve Git blaming for the analysis #1247
Conversation
kirill-knize-sonarsource
force-pushed
the
feature/kk/git-blame-time-window
branch
from
bd862b3 to
c0d21fe
Compare
kirill-knize-sonarsource
force-pushed
the
feature/kk/git-blame-time-window
branch
2 times, most recently
from
94fe8c7 to
9ea9367
Compare
kirill-knize-sonarsource
force-pushed
the
feature/kk/git-blame-time-window
branch
from
9ea9367 to
625d54b
Compare
kirill-knize-sonarsource
force-pushed
the
feature/kk/git-blame-time-window
branch
2 times, most recently
from
1b25257 to
5c8cf22
Compare
kirill-knize-sonarsource
force-pushed
the
feature/kk/git-blame-time-window
branch
from
5c8cf22 to
f97e0ce
Compare
|
| @@ -62,6 +62,8 @@ static NewCodeDefinition withNumberOfDaysWithDate(int days, long thresholdDate) | |||
| return new NewCodeNumberOfDaysWithDate(days, thresholdDate); | |||
| } | |||
|
|
|||
| long getThresholdDate(); | |||
There was a problem hiding this comment.
Why not returning an Instant from here? You can make the conversion later when you use it
There was a problem hiding this comment.
I think threshold date is maybe not precise enough. I am bad at naming, but what about something like "period start date"?
There was a problem hiding this comment.
Nitpick: I would move the method somewhere else, to not be in the middle of with* methods.
| @@ -116,6 +118,11 @@ public boolean isSupported() { | |||
| return true; | |||
| } | |||
|
|
|||
| @Override | |||
| public long getThresholdDate() { | |||
There was a problem hiding this comment.
Shouldn't this method also be implemented in NewCodeDefinitionWithDate? Maybe it's the case.
EDIT: I see this abstract class is not used at all, maybe we could remove it?
| @@ -189,6 +192,11 @@ public static SonarLintBlameResult blameFromNativeCommand(Path projectBaseDir, S | |||
| throw new IllegalStateException("There is no native Git available"); | |||
| } | |||
|
|
|||
| private static String getBlameHistoryWindow(long thresholdDate) { | |||
| var blameLimit = Instant.ofEpochMilli(thresholdDate); | |||
There was a problem hiding this comment.
Related to what I said earlier, we create Instants that we convert to a timestamp, then back to an Instant
SLCORE-1142