On Wed, Jan 16, 2013 at 9:11 PM, Jeff King <p...@peff.net> wrote: >> is_forwardable() did solve a UI issue. Previously all instances where >> old is not reachable by new were assumed to be addressable with a >> merge. is_forwardable() attempted to determine if the concept of >> forwarding made sense given the inputs. For example, if old is a blob >> it is useless to suggest merging it. > > I think it makes sense to mark such a case as different from a regular > non-fast-forward (because "git pull" is not the right advice), but: > > 1. is_forwardable should assume a missing object is a commit not to > regress the common case; otherwise we do not show the pull advice > when we probably should, and most of the time it is going to be a > commit
Yes, obviously this was a bug, thus the use of "attempted" above. It would have been better to assume a missing 'old' was potentially forwardable to present the user with the most helpful advice. > 2. When we know that we are not working with commits, I am not sure > that "already exists" is the right advice to give for such a case. > It is neither "this tag already exists, so we do not update it", > nor is it strictly "cannot fast forward this commit", but rather > something else. But the reference already existing in the remote is a substantial reason for not allowing the push in all of these cases. You can break this out further if you like to explain why the specific reference shouldn't be moved on the remote, but this is even more complicated a simple "is old reachable from new?" test. Chris -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
