feat: add add subcommand - #16
ianthomas23 merged 10 commits into
Conversation
Sorry, something went wrong.
|
|
||
| void index_wrapper::add_impl(std::vector<std::string> patterns) | ||
| { | ||
| git_strarray array{new char*[patterns.size()], patterns.size()}; |
There was a problem hiding this comment.
This leaks. I think it would be worth having a wrapper of git_strarray, that correctly manages the memory and use it here. It will be useful for other commands anyway.
Sorry, something went wrong.
|
|
||
| void index_wrapper::add_impl(std::vector<std::string> patterns) | ||
| { | ||
| git_strarray_wrapper array=git_strarray_wrapper(patterns); |
There was a problem hiding this comment.
| git_strarray_wrapper array=git_strarray_wrapper(patterns); | |
| git_strarray_wrapper array{patterns}; |
Sorry, something went wrong.
| #define GIT2CPP_VERSION_MAJOR 0 | ||
| #define GIT2CPP_VERSION_MINOR 0 | ||
| #define GIT2CPP_VERSION_PATCH 1 | ||
| #define GIT2CPP_VERSION_PATCH 2 |
There was a problem hiding this comment.
Why updating the version?
Sorry, something went wrong.
There was a problem hiding this comment.
The 0.0.2 has been released but we forgot to change it there, so it's to catch up.
Sorry, something went wrong.
|
|
||
| private: | ||
| bool all_flag = false; | ||
| std::vector<std::string> add_files; |
There was a problem hiding this comment.
Convention: data member names should start with m_ (or p_) so it is easy to distinguish them from local variables.
Sorry, something went wrong.
There was a problem hiding this comment.
m_ stands for "member" and p_ stands for what ?
Sorry, something went wrong.
There was a problem hiding this comment.
pointer
Sorry, something went wrong.
| def test_add(git2cpp_path, all_flag): | ||
| with open("./test/mook_file.txt", "x") as f: | ||
| pass | ||
| f.close() |
There was a problem hiding this comment.
This line isn't needed as the f will be automatically closed when the with ... block is exited. The same applies to 4 lines below.
Sorry, something went wrong.
|
|
||
| os.remove("./test/mook_file.txt") | ||
| os.remove("./test/mook_file_2.txt") | ||
| subprocess.run(cmd_add, capture_output=True, text=True) |
There was a problem hiding this comment.
Is this line just to undo the add, to leave the test directory at the end the same as it was at the start? If so can you add a comment saying this, otherwise it will look suspicious in future to have this line without any following assert.
Sorry, something went wrong.
There was a problem hiding this comment.
Yes, that's the reason why ! I'll write a comment.
Sorry, something went wrong.
ianthomas23
left a comment
There was a problem hiding this comment.
Thanks @SandrineP!
Sorry, something went wrong.
Add the git add sub-command, with the --all flag.