On Tue, Apr 10, 2012 at 02:09:17PM -0700, Junio C Hamano wrote: > Neil Horman <nhorman@xxxxxxxxxxxxx> writes: > > > ... What do you say to my proposal regarding the splitting of the device > > dependent on how we were executed? It seems we can't use a single advice string > > in this case, as no matter which we choose there is a use case in which it fails > > to make sense. > > When your cherry-pick got --allow-empty, it should pass --allow-empty to > its inner invocation of commit if it is picking an originally empty > commit, and this advice will not trigger because commit will happily > commit the no-change change. > > When your cherry-pick got --allow-empty but not --keep-unnecessary-commit, > its inner invocation of commit must not pass --allow-empty if it is _not_ > picking an originally empty commit. Then the inner commit will fail if it > auto resolves to no change, and the user sees the advice. The current > advice text is appropriate for this case. > > When your cherry-pick did not get either of these flags, its inner > invocation of commit must not pass --allow-empty. The user sees the > advice when the auto resolved result matches HEAD from the commit invoked > by cherry-pick. The current advice text is fine for this case, as we say > "possibly", not "we definitely know it was due to conflict resolution". > > Or your cherry-pick may have failed due to a conflict, regardless of the > options like --allow-empty or --keep-unnecessary-commit given to it, and > the user may have run commit after resolving the conflict. The current > advice text is fine for this case, too, as we say "possibly", and it > indeed is what just happened. > > So I do not think you need to change anything with respect to the advice > message. > > Am I missing some other cases? > No, you covered all the cases, but I disagree with your assertion that the advice is correct (or at least optimal) in any of these cases. If a cherry-pick without any options is preformed and the commit is empty (regardless of the reason), the advice given is that git commit --allow-empty should be used. With the addition of these new options, thats not true any longer. Instead of using git commit --allow-empty, you can use git cherry-pick --allow-empty. Given your description above though, I'm ok rolling this back. Regardless of my disagreement, git commit --allow-empty still works just as well after the addition of these options, so while I still think its awkward to give git commit advice on the result of a git cherry-pick operation, its not any more problematic than the issues you've pointed out with my change. I'll roll this back in my next version and will look for a way to provide better advice in the future. Regards Neil > -- 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