On Mon, Aug 01, 2016 at 02:12:34PM -0400, Jeff King wrote: > Before I switched to "reset at the beginning" in my original patch, I > also noticed this issue, but decided the other way: to only reset after > a successful creation. > > I think in most cases it wouldn't matter anyway, because the caller will > generally abort as soon as this returns failure anyway. But I wondered > about something like: > > author = prepare_author_info(); > if (commit_tree_extended(..., author, ...) < 0) { > /* oops, we failed. Do a thing and try again. */ > possible_fix(); > if (commit_tree_extended(..., author, ...) < 0) > die("giving up"); > } > > In the second call (but only the second call!) the committer and author > can diverge. To be clear, I checked all of the callers and nobody actually does this. Every caller proceeds straight to a die() except the one in notes_cache_write(), which silently ignores error (which is the correct thing to do). This is more "it seems like a fragile pattern to me". -Peff -- 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