On Tuesday, February 16, 2016 01:54:48 PM Junio C Hamano wrote: > "Philip Oakley" <philipoakley@xxxxxxx> writes: > > >> It appeared that the conditional for 'Reject an attempt to record a > >> non-merge empty commit without * explicit --allow-empty.' could be > >> simplified after adding this patch. > >> > >> This change can't be propagated to the conditional because it allows > >> a commit that was previously disallowed. > > This last sentence sounds somewhat worrysome. Does that mean some > commit that was previously disallowed (which ones?) is still > forbidden by "commit" without "--dry-run" (which is correct--we are > not interested in changing the behaviour of the main codepath), but > "--dry-run", even with this update, will say "OK you will make a > meaningful commit" by exiting with 0 for such disallowed commit? I tried to think of a better set of wording. Finally I decided to make it part of the note rather than the commit message so that it could be debated as part of the review but not be part of the commit record for the line being changed. The patch doesn't change behaviour other than the dry-run return code which now matches the return code of commit. The one line change is not changing the main code path behaviour The main code path for the case being fixed executes through the main code path successfully returning zero. The ''--dry-run' was predicitng failure if a script was checking the return code, but successs if looking at the messages. The final couple of paragraphs explain why I chose not to change the if() statement. The reason I didn't is so that expected behaviour is maintained. The condition that can not be removed in the if is the 'whence != FROM_MERGE'. Removing that caused t7502 to generate errors. Therefore I left ' if (!commitable && whence != FROM_MERGE && !allow_empty && !(amend && is_a_merge(current_head)))' in the commit.c file. -- 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