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.

Sign up for GitHub

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

Jump to bottom

Use GitBlameLineInfo for processing git blame output in aggregateBlameAuthorModifiedAndDateInfo #2196

Closed
gok99 opened this issue Apr 27, 2024 · 5 comments · Fixed by #2232
Closed

Use GitBlameLineInfo for processing git blame output in aggregateBlameAuthorModifiedAndDateInfo #2196

gok99 opened this issue Apr 27, 2024 · 5 comments · Fixed by #2232
Assignees
@logical-1985516
Labels
a-Backend a-CodeQuality c.Enhancement d.FirstTimers

Comments

@gok99
Copy link
Contributor

gok99 commented Apr 27, 2024

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

RepoSense/src/main/java/reposense/authorship/FileInfoAnalyzer.java

Lines 158 to 183 in bf78bf2

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.
@gok99 gok99 mentioned this issue Apr 27, 2024
Merged
@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 logical-1985516 mentioned this issue Jul 3, 2024
Merged
@logical-1985516

This comment was marked as outdated.

Sign in to view
@logical-1985516
Copy link
Contributor

logical-1985516 commented Jul 6, 2024 •
edited
Loading

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.

gok99 pushed a commit that referenced this issue Jan 6, 2025
@logical-1985516
[#2196] Use git blame line info for aggregate blame author modified a…
e53c52c 
…nd date info (#2232)

`aggregateBlameAuthorModifiedAndDateInfo` is hard to maintain, due to
complex string wrangling and seemingly magic numbers. Let's fix these using
`GitBlameLineInfo` introduced in #2140.

- Use blameLine to abstract away the string wrangling and improve
understandability. Magic numbers are also replaced to improve code quality.    

- In order to use blameLine however, it is also changed to use 
author-time instead of commit-time, as there is a discrepancy between the two,
causing some test cases to fail. 

 - The naming of the timestamp field in GitBlameLineInfo is changed to
reflect that it is in seconds, as author-time is in seconds.
@github-project-automation github-project-automation bot moved this to Closed/Completed in RepoSense Roadmap Jan 6, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@logical-1985516 logical-1985516

Labels
a-Backend a-CodeQuality c.Enhancement d.FirstTimers
Projects
RepoSense Roadmap
Status: Closed/Completed
Milestone
No milestone
3 participants
@gok99 @ckcherry23 @logical-1985516