| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
Codecov Report❌ Patch coverage is 98.90110% with 1 line in your changes missing coverage. Please review.
@@ Coverage Diff @@
## main #106 +/- ##
==========================================
+ Coverage 86.18% 86.83% +0.65%
==========================================
Files 60 60
Lines 2186 2340 +154
Branches 244 276 +32
==========================================
+ Hits 1884 2032 +148
- Misses 302 308 +6 ☔ View full report in Codecov by Sentry.
|
Sorry, something went wrong.
I looked at the extra newlines, and I thought that there was always a trailing one at the end of the message in git2cpp. So I changed the code a bit and in the tests I made, there was only a newline at the very end that is not there with git. The rest was identical. So I thought it was fine, but apparently not ! I'll have another look. |
Sorry, something went wrong.
| } | ||
| } | ||
|
|
||
| git_strarray_dispose(&tag_names); |
There was a problem hiding this comment.
This is not exception-safe. However, we cannot use the git_strarray_wrapper here, since this class actually wraps a git_strarray whose elements point to already allocating strings (and thus, git_strarray_dispose is not called in the destructor to avoid double deletion).
Which leads me to think that the current git_strarray_wrapper is probably badly named. And that we should have a git_strarray_wrapper that frees both git_strarray elements and the array itself.
But this implies refactoring and goes beyond the scope of this PR, so let's add a TODO here and open an issue to track it when we merge this PR.
Sorry, something went wrong.
| std::string branch_name; | ||
| branch_name = branch->name(); |
There was a problem hiding this comment.
| std::string branch_name; | |
| branch_name = branch->name(); | |
| std::string branch_name = branch->name(); |
Sorry, something went wrong.
There was a problem hiding this comment.
I wrote it this way because it's not happy with the one line option:
error: conversion from 'std::string_view' {aka 'std::basic_string_view<char>'} to non-scalar type 'std::string' {aka 'std::__cxx11::basic_string<char>'} requested
Sorry, something went wrong.
There was a problem hiding this comment.
How about
std::string branch_name(branch->name());instead, which works for me on macos?
Sorry, something went wrong.
There was a problem hiding this comment.
I missed that the return type of branch::name was std::string_view. In that case the correct solution is the line from Ian.
Sorry, something went wrong.
| return tags; | ||
| } | ||
|
|
||
| std::vector<std::string> get_branches_for_commit(repository_wrapper& repo, git_branch_t type, const git_oid* commit_oid, const std::string exclude_branch) |
There was a problem hiding this comment.
commit_oid should be passed by reference insteadof pointer.
Sorry, something went wrong.
| } | ||
| }; | ||
|
|
||
| commit_refs get_refs_for_commit(repository_wrapper& repo, const git_oid* commit_oid) |
There was a problem hiding this comment.
commit_oid should be passed by reference instead of pointer.
Sorry, something went wrong.
| return refs; | ||
| } | ||
|
|
||
| void print_refs(commit_refs refs) |
There was a problem hiding this comment.
refs should be passed by constant reference.
Sorry, something went wrong.
There was a problem hiding this comment.
The whitespace looks excellent now thanks, and I've checked all of Johan's comments too and this is ready to merge.
I've left the one comment about the branch_name string, feel free to make that change or not.
Sorry, something went wrong.
| } | ||
|
|
||
| void print_commit(const commit_wrapper& commit, std::string m_format_flag) | ||
| std::vector<std::string> get_tags_for_commit(repository_wrapper& repo, const git_oid* commit_oid) |
There was a problem hiding this comment.
The commit_oid parameter should be passed by reference.
Sorry, something went wrong.
|
I've confirmed that the extra 2 proposed changes have been implemented, so I am merging. |
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Fix #90
Add tracking info for log subcommand