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
Feature/2952/automate gitlogparser #3041
Conversation
…er' into feature/2952/automate-gitlogparser
[CodeCharta Analysis] Kudos, SonarCloud Quality Gate passed! |
[CodeCharta Visualization] Kudos, SonarCloud Quality Gate passed! |
parserStrategy: LogParserStrategy, | ||
metricsFactory: MetricsFactory, | ||
containsAuthors: Boolean, | ||
silent: Boolean = false | ||
): Project { | ||
val namesInProject = readFileNameListFile(pathToNameTree) | ||
val encoding = guessEncoding(pathToLog) ?: "UTF-8" | ||
val namesInProject = readFileNameListFile(gitLsFile) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would always try to not use a abbreviation/acronym in variable names. By that it is mostly hard for new developers to understand the purpose of the variable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I remember talking about this with @Hall-Ma, we a lot of back and forth about this name too. Something like gitFiles or trackedFiles(List) would make the datatype hard to read/guess at a glance. We settled on this since we thought that the "ls" command is well known enough and can be inferred from both the docs and the command description.
I will try think of a new and better name though! :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't get the point that you mean the list command by Ls
. :-)
I admit it is a bit tricky to combine the type "File" with the variable name. So I would just sugggest something like listOfRepositoryFiles
. But this is nit picking. It is fine for me to keep it for now.
required = true, | ||
description = ["list of all file names in current git project"] | ||
) | ||
private var gitLsFile: File? = null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same effect here.
return tempGitLog | ||
} | ||
|
||
private fun createGitLsFile(repoPath: Path): File { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here.
Automate the gitlogparser further, to save the users from running even more shell commands
Issue: #2952
Description
When using the GitLogParser, the user needs to execute two commands before being able to actually execute the parser. This is clunky and not very user-friendly. This PR modifies the GitLogParser to now use two subcommands instead - repo-scan and log-scan - which execute the necessary git commands automatically and provide more logical option names for the existing functionality respectively.
The PR also upgrades the testing library to JUnit5, and removes redundant assertion libraries (hamcrest and hamkrest).