On Tue, Aug 29, 2017 at 8:58 AM, Kevin Willford <kewillf@xxxxxxxxxxxxx> wrote: >> >> On Mon, Aug 28, 2017 at 02:28:28PM -0600, Kevin Willford wrote: >> >> > The return value of the get_files_dirs call is never being used. >> > Looking at the history of the file and it was originally only >> > being used for debug output statements. Also when >> > read_tree_recursive return value is non zero it is changed to >> > zero. This leads me to believe that it doesn't matter if >> > read_tree_recursive gets an error. >> >> Or that the function is buggy. :) > > That was one of my questions as well. Should read_tree_recursive > be propagating a -1 and merge_trees be checking for that and bail > when the call to get_files_dirs return is < 0? I made a commit with > this change and ran the tests and they all still passed so either this > return really doesn't matter or there are not sufficient tests covering > it. > > I went with this change because it was not changing any of the > current functionality and if we find a case where it matters that > read_tree_recursive fails due to bad tree or something else we > can address it then. > >> >> I'm tempted to say that we should probably die() when >> read_tree_recursive fails. This should only happen if we fail to parse >> the tree, or if our callback (save_files_dirs here) returns failure, and >> the latter looks like it never happens. >> >> > Since the debug output has been removed and the caller isn't >> > checking the return value there is no reason to keep calulating >> > and returning a value. >> >> Agreed, and I'm happy to see dead code go. >> >> Minor nit: s/calulating/calculating/ in your commit message. > > When will that spell checker for git messages be ready? ;) Different workflows need different setups apparently. (me, as a heavy user of git-gui, likes the builtin spell checker, though I need to find a way to train it to accept git lingo, such as "submodule", or "gitlink") Maybe: https://tarasalenin.wordpress.com/2010/09/11/configure-git-gui-spell-checker-on-windows/ If you do not use git-gui... you are at the merci of your $EDITOR ($GIT_EDITOR, or core.editor) to have spell checking.