Experiments with wrapping git CLI command with CliWrap
#243
Conversation
![codeclimate[bot]](https://avatars.githubusercontent.com/in/9403?s=60&v=4){kind=small}
Freshli/GitCliRepository.cs
Outdated
| RepositoryRoot = repositoryRoot; | ||
| } | ||
|
|
||
| public Dictionary<DateTime, string> LogEntriesFor(string path) { |
There was a problem hiding this comment.
Method LogEntriesFor has 30 lines of code (exceeds 25 allowed). Consider refactoring.
This includes some logic to retry the HTTP request for the .atom xml file. The retry uses a logarithmic backoff starting at 100ms, and attempts to retry 5 times before giving up.
|
@mscottford if I understand correctly removing the libgit2sharp library will require any consumers of this NuGet package to have Git installed. Is that extra step, requiring Git to be installed in a path the NuGet package can find, easier then the trouble we are having with libgit2sharp? What are the issues we are having with libgit2sharp? |
Yes, the
Good question. First up, support for this package is waning and there is a need for additional maintainers (see: libgit2/libgit2sharp#1834). I don't feel like we have the resources that would be needed to get involved, and behavior that we need from the library is rather limited. We've also logged some past issues with this library in the past.
Next up are some comments that I've seen while reading through the project's issues.
|
Freshli/GitCliRepository.cs
Outdated
| RepositoryRoot = repositoryRoot; | ||
| } | ||
|
|
||
| public Dictionary<DateTimeOffset, string> LogEntriesFor(string path) { |
There was a problem hiding this comment.
Method LogEntriesFor has 30 lines of code (exceeds 25 allowed). Consider refactoring.
|
Code Climate has analyzed commit d84d05b and detected 2 issues on this pull request. Here's the issue category breakdown:
View more on Code Climate. |
There was a bug where that was causing the start of the date range to be older than the oldest revision in the file history. Meaning that the file would not have existed at the start of the date range that was being used for analysis. This changes the logic so that the oldest file date is always the starting point of the range, and then range is built in one month increments starting with the start of the month that immediately follows the creation of the first file in the history.
The logic was starting with the oldest file and then incrementing one month at a time from there. The timezone of that value might not match the timezone that has been passed in via `asOf`. With this change, we assume that the caller wants the results to be in the same timezone offset that's specified in the `asOf` value. Before this change, all results would have been in the timezone of that first commit.
|
This train-wreck of a monster pull-request is ready for review. At the moment, the class that wraps the Since this is such a mess, one option is to break it up into smaller pull requests that can be evaluated easier. Or we can just move forward and accept the messiness in the commit history. |
|
Code Climate has analyzed commit e85b623 and detected 36 issues on this pull request. Here's the issue category breakdown:
The test coverage on the diff in this pull request is 92.6% (50% is the threshold). This pull request will bring the total coverage in the repository to 92.0% (1.5% change). View more on Code Climate. |
|
This work is being abandoned in favor of the approach taken in the Freshli-CLI project. |
We've had some challenges with libgit2sharp. This experiment is trying to evaluate the feasibility of breaking our dependency with that package and instead, just using the
gitCLI to get the information that we need.