Johannes Schindelin venit, vidit, dixit 06.02.2009 16:38: > Hi, > > On Fri, 6 Feb 2009, Michael J Gruber wrote: > >> Currently, git-notes barfs when asked to show an empty (i.e. >> non-existing) note. Change this to explicitely say there is none. >> --- >> git-notes.sh | 2 ++ >> t/t3301-notes.sh | 2 +- >> 2 files changed, 3 insertions(+), 1 deletions(-) >> >> diff --git a/git-notes.sh b/git-notes.sh >> index bfdbaa8..9cbad02 100755 >> --- a/git-notes.sh >> +++ b/git-notes.sh >> @@ -58,6 +58,8 @@ edit) >> "$GIT_NOTES_REF" $NEW_HEAD $CURRENT_HEAD >> ;; >> show) >> + git rev-parse -q --verify "$GIT_NOTES_REF":$COMMIT > /dev/null || >> + die "No note for commit $COMMIT." > > This looks good. > >> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh >> index 4900dca..81d5028 100755 >> --- a/t/t3301-notes.sh >> +++ b/t/t3301-notes.sh >> @@ -36,7 +36,7 @@ test_expect_success 'need valid notes ref' ' >> ' >> >> # 1 indicates caught gracefully by die, 128 means git-show barfed >> -test_expect_failure 'handle empty notes gracefully' ' >> +test_expect_success 'handle empty notes gracefully' ' >> git notes show || test 1 = $? > > Completely forgot to mention that I think you want to use test_must_fail > here. What does test_must_fail mean? I don't see it in t/README. > And maybe you want to be more explicit, by specifying which > commit's notes are expected not to be there. Well, HEAD's notes. I can say HEAD explicitly, of course. > We would not want the test to succeed for all the wrong reasons, would we? Well, I could test for the friendly "No note for commit" message on output, I just thought that is fragile. You see, I did not even want to write a test for such a simple patch... ;) OK, do we agree on the following intended behaviour for git notes show: - return 0 if a note can be shown - return 1 if there is none (i.e. die gracefully) - return something else (i.e. die fatally) if something really bad happens Then I should rewrite the test to check for "1" and only "1". Now, the coffee... Michael -- 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
