Jonathan Nieder wrote: > My "alternatively" was bogus --- lookup_commit_reference takes a (raw) > full commit name as its argument. > > I dunno. The question was not actually rhetorical --- I just meant > that it's worth thinking about these cases. There are a few cases: > > - missing object > - object is present but corrupt > - object is a blob, not a commit > > In the second case, there's an error message printed describing the > problem, but in the other two there isn't. The other callers tend to > say "not a valid <foo>" or "could not lookup commit <foo>, so I guess > > error: .git/sequencer/todo:5: not a valid commit: 78a89f493 > > would be fine. > > Except that this focusses on the .git/sequencer/todo filename which > would leave the person debugging astray. It is not that > .git/sequencer/todo contains a typo (that would have been caught by > get_sha1), but that it referred to a bad object or non-commit. Maybe > something in the direction of > > error: cannot pick 78a89f493 because it is not a valid commit > > would be more helpful. > > Is this the right moment to report that error? Will the operator be > happy that we errored out right away before cherry-picking anything > and wasting the human's time in assisting with that process, or will > she be unhappy that inability to do something later that she might > have been planning on skipping anyway prevented making progress right > away? (I'm not sure what the best thing to do is --- I guess some > advice like > > hint: to abort, use cherry-pick --abort > hint: to skip or replace that commit, use cherry-pick --edit > > would help.) Ignoring this in the re-roll: I'd be inclined to put these changes in a separate series that begins with bolting on some advice configuration to the sequencer. Thanks for thinking about these things. -- Ram -- 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