Skip to content
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

Use GitBlameLineInfo for processing git blame output in aggregateBlameAuthorModifiedAndDateInfo #2196

Open
gok99 opened this issue Apr 27, 2024 · 5 comments · May be fixed by #2232
Open

Use GitBlameLineInfo for processing git blame output in aggregateBlameAuthorModifiedAndDateInfo #2196

gok99 opened this issue Apr 27, 2024 · 5 comments · May be fixed by #2232

Comments

@gok99
Copy link
Contributor

gok99 commented Apr 27, 2024

What feature(s) would you like to see in RepoSense

for (int lineCount = 0; lineCount < blameResultLines.length; lineCount += 5) {
String commitHash = blameResultLines[lineCount].substring(0, FULL_COMMIT_HASH_LENGTH);
String authorName = blameResultLines[lineCount + 1].substring(AUTHOR_NAME_OFFSET);
String authorEmail = blameResultLines[lineCount + 2]
.substring(AUTHOR_EMAIL_OFFSET).replaceAll("<|>", "");
Long commitDateInMs = Long.parseLong(blameResultLines[lineCount + 3].substring(AUTHOR_TIME_OFFSET)) * 1000;
LocalDateTime commitDate = LocalDateTime.ofInstant(Instant.ofEpochMilli(commitDateInMs),
config.getZoneId());
Author author = config.getAuthor(authorName, authorEmail);
if (!fileInfo.isFileLineTracked(lineCount / 5) || author.isIgnoringFile(filePath)
|| CommitHash.isInsideCommitList(commitHash, config.getIgnoreCommitList())
|| commitDate.compareTo(sinceDate) < 0 || commitDate.compareTo(untilDate) > 0) {
author = Author.UNKNOWN_AUTHOR;
}
if (config.isLastModifiedDateIncluded()) {
if (config.isShallowCloningPerformed()) {
logger.warning(String.format(
MESSAGE_SHALLOW_CLONING_LAST_MODIFIED_DATE_CONFLICT, config.getRepoName()));
}
fileInfo.setLineLastModifiedDate(lineCount / 5, commitDate);
}
fileInfo.setLineAuthor(lineCount / 5, author);
}

aggregateBlameAuthorModifiedAndDateInfo in FileInfoAnalyzer.java does some manual string wrangling of git blame output which is hard to interpret, contains magic numbers and breaks the git blame abstraction.

If possible, describe the solution

#2140 Introduces GitBlameLineInfo, returned by blameLine, which encapsulates this string wrangling logic. We can replace this manual string processing by using this method in aggregateBlameAuthorModifiedAndDateInfo.

@logical-1985516
Copy link
Contributor

May I try this issue?

@ckcherry23
Copy link
Member

@logical-1985516 Sure, please do let us know if you have any questions!

@logical-1985516

This comment was marked as outdated.

@logical-1985516
Copy link
Contributor

logical-1985516 commented Jul 6, 2024

Hi, should we change commit-time in processGitBlameResultLine to author-time? Originally, author-time is used, and the discrepancy between author-time and commit-time has caused some test cases to fail.
I have also noticed that timestamp for author-time is in seconds. Should we change timestampMilliseconds in GitBlameLineInfo to timestampSeconds?

@ckcherry23
Copy link
Member

@logical-1985516 Thanks for investigating what caused the test cases to fail. You can make the changes you described.

Also, since you already have a PR open, you can discuss this problem in PR #2232 itself, as that gives us the most context.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
3 participants