Hi, Ronnie Sahlberg wrote: > Update repack_without_refs to take an err argument and update it if there > is a failure. Pass the err variable from ref_transaction_commit to this > function so that callers can print a meaningful error message if _commit > fails due to a problem in repack_without_refs. > > Add a new function unable_to_lock_message that takes a strbuf argument and > fills in the reason for the failure. > > In commit_packed_refs, make sure that we propagate any errno that > commit_lock_file might have set back to our caller. > > Signed-off-by: Ronnie Sahlberg <sahlberg@xxxxxxxxxx> > --- > cache.h | 2 ++ > lockfile.c | 21 ++++++++++++--------- > refs.c | 25 +++++++++++++++++++------ > 3 files changed, 33 insertions(+), 15 deletions(-) I don't want to sound like a broken record, but this still has the same issues I described before at http://thread.gmane.org/gmane.comp.version-control.git/250197/focus=250309. The commit message or documentation or notes after the three dashes above could explain what I missed when making my suggestions. Otherwise I get no reality check as a reviewer, other reviewers get less insight into what's happening in the patch, people in the future looking into the patch don't understand its design as well, etc. As a general rule, that is a good practice anyway --- even when a reviewer was confused, what they got confused about can be an indication of where to make the code or design documentation (commit message) more clear, and when reposting a patch it can be a good opportunity to explain how the patch evolved. What would be wrong with the line of API documentation and the TODO comment for a known bug I suggested? If they are a bad idea, can you explain that so I can learn from it? Or if they were confusing, would you like a patch that explains what I mean? Jonathan -- 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