2009/9/28 Eric Kow <[email protected]> > Hey all, > > Kamil's patch could use some attention. Are there any reviewers free > who could look at it? >
I'm looking at it now. I can't say much about the tests, but the rest of the code looks good to me and I'm going to read up on the bug tickets to try and understand things. http://bugs.darcs.net/issue1620 -- A bit terse, but I get the gist of it. [accept issue1620: record lies about leaving the logfile] I don't see anything that looks funny, but the shell scripts are one of my weak points. [accept issue1618: amend should preserve the logfile] Again, near as I can tell it's good. [remove trailing whitespace] I wish no one cared about trailing whitespace, but since people clearly do, I guess it's good we're killing it off bit by byte. [Resolve issue 1618: preserve log on amend failure] Looks good. [camelCase clarify_errors] Yay! [resolve issue1620: amend lies about living logfile] A little bit of refactoring and seems to do what it says it should do. It seems we may have a problem: Running issue1618-amend-preserve-logfile.sh ... failed. Probable reason :. ../tests/lib # This is a -*- sh -*- library. set -vex ## I would use the builtin !, but that has the wrong semantics. not () { "$@" && exit 1 || :; } # trick: OS-detection (if needed) abort_windows () { if echo $OS | grep -i windows; then echo This test does not work on Windows exit 200 fi } pwd() { ghc --make "$TESTS_WD/hspwd.hs" "$TESTS_WD/hspwd" } # switch locale to latin9 if supported if there's a locale command, skip test # otherwise switch_to_latin9_locale () { if ! which locale ; then echo "no locale command" exit 200 # skip test fi latin9_locale=`locale -a | grep @euro | head -n 1` if [ -z "$latin9_locale" ]; then echo "no latin9 locale found" exit 200 # skip, we can't switch away from UTF-8 fi echo "Using locale $latin9_locale" export LC_ALL=$latin9_locale echo "character encoding is now `locale charmap`" } rm -rf R; mkdir R; cd R + rm -rf R + mkdir R + cd R darcs init + darcs init darcs setpref test false + darcs setpref test false Changing value of test from '' to 'false' darcs record -am foo --no-test + darcs record -am foo --no-test Finished recording patch 'foo' export DARCS_EDITOR="echo 'new log' > " + export 'DARCS_EDITOR=echo '\''new log'\'' > ' + DARCS_EDITOR='echo '\''new log'\'' > ' echo y | not darcs amend -p foo --edit-long-comment 2> out + echo y + not darcs amend -p foo --edit-long-comment Mon Sep 28 22:10:23 PDT 2009 tester * foo Shall I amend this patch? [yNvpxq], or ? for help: Running test... Test failed! # the msg has the format: "Logfile left in filenamehere." LOGFILE=`grep "Logfile left in" out | sed "s/Logfile left in //" | sed s/.$//` grep "Logfile left in" out | sed "s/Logfile left in //" | sed s/.$// ++ grep 'Logfile left in' out ++ sed 's/Logfile left in //' ++ sed 's/.$//' + LOGFILE= echo $LOGFILE + echo test -e "$LOGFILE" + test -e '' Running issue1620-record-lies-about-leaving-logfile.sh ... failed. Probable reason :. ../tests/lib # This is a -*- sh -*- library. set -vex ## I would use the builtin !, but that has the wrong semantics. not () { "$@" && exit 1 || :; } # trick: OS-detection (if needed) abort_windows () { if echo $OS | grep -i windows; then echo This test does not work on Windows exit 200 fi } pwd() { ghc --make "$TESTS_WD/hspwd.hs" "$TESTS_WD/hspwd" } # switch locale to latin9 if supported if there's a locale command, skip test # otherwise switch_to_latin9_locale () { if ! which locale ; then echo "no locale command" exit 200 # skip test fi latin9_locale=`locale -a | grep @euro | head -n 1` if [ -z "$latin9_locale" ]; then echo "no latin9 locale found" exit 200 # skip, we can't switch away from UTF-8 fi echo "Using locale $latin9_locale" export LC_ALL=$latin9_locale echo "character encoding is now `locale charmap`" } rm -rf R; mkdir R; cd R + rm -rf R + mkdir R + cd R export DARCS_EDITOR="echo 'a log' > " + export 'DARCS_EDITOR=echo '\''a log'\'' > ' + DARCS_EDITOR='echo '\''a log'\'' > ' darcs init + darcs init darcs setpref test false + darcs setpref test false Changing value of test from '' to 'false' echo yy | not darcs record -m foo -a --edit-long-comment 2> out + echo yy + not darcs record -m foo -a --edit-long-comment Running test... Test failed! # the msg has the format: "Logfile left in filenamehere." LOGFILE=`grep "Logfile left in" out | sed "s/Logfile left in //" | sed s/.$//` grep "Logfile left in" out | sed "s/Logfile left in //" | sed s/.$// ++ grep 'Logfile left in' out ++ sed 's/.$//' ++ sed 's/Logfile left in //' + LOGFILE=darcs-record-0 test -e "$LOGFILE" + test -e darcs-record-0 And at the very end: Some tests failed: issue1618-amend-preserve-logfile.sh issue1620-record-lies-about-leaving-logfile.sh setup: user error (Tests failed) Any ideas? Thanks, Jason
_______________________________________________ darcs-users mailing list [email protected] http://lists.osuosl.org/mailman/listinfo/darcs-users
