On 09/16/2014 10:57 PM, Jonathan Nieder wrote: > Michael Haggerty wrote: > >> The ambiguity didn't have any ill effects, because lock_file objects >> cannot be removed from the lock_file_list anyway. But it is >> unnecessary to leave this behavior inconsistent. > > Nit: commit messages usually use present tense for current behavior > (and imperative for the behavior after the patch). OK, will change. > More importantly, the above + the diffstat don't leave me very happy > about the change. > > Can you spell out more about the intended effect? E.g., is this about > making sure other code is appropriately exercised to tolerate the > signal handler even when there are a lot of errors (which should make > debugging easier) or something? This patch is mostly about two things: * Making the state diagram for lock_file objects easier to document and reason about. * Defining a clear moment when the lock_file object is initialized, early in the function. This will be useful later when its filename field is turned into a strbuf and requires nontrivial initialization before it can even be used to construct the filename of the lockfile. By "diffstat" I assume you mean that some lines were added to the initialization. This is also to make the state diagram of lock_objects very clear. I will rewrite the commit message to try to explain the commit's purpose better. Let me know if you still think that the commit is unjustified. Michael -- Michael Haggerty mhagger@xxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html