Patrick Steinhardt <ps@xxxxxx> writes: >> Hmph, in other places I did >> >> if (!the_hash_algo) >> repo_set_hash_algo(the_repository, GIT_HASH_SHA1); >> >> to find the case where we need a reasonable default. >> >> Is there a practical difference? If there isn't we should >> standardise one and use the same test consistently everywhere. >> ... > > To the best of my knowledge there isn't. What I prefer about my approach > is that it explicitly points out that this is conditional on whether or > not we have a repository. But in the end I don't mind much which of both > versions we use. Ah, that makes sense, and it is quite subjective but makes certain sense. The reason I prefered to check "the_hash_algo" is very much the opposite. In this particular decision to call (or not call) set_hash_algo(), we only care if the_hash_algo is not yet set, and that is why the_hash_algo is checked. Specifically, we do not care *why* it is still unset; in the current codebase, the most likely reason why we do not have the_hash_algo set might be that we haven't found the repository yet, but we do not have to rely on that assumption to hold true. It would help maintainability into the future where the_hash_algo is set already before we come here when outside a repository, or vice versa.
