On Wed, Apr 09, 2008 at 08:55:31AM +0200, Johannes Sixt wrote:
quoted text
> Bryan Donlan schrieb:
> > -dotest=$GIT_DIR/.dotest-merge
> > +dotest="$GIT_DIR/.dotest-merge"
> 
> This is not strictly necessary: The RHS expression of an assignment does
> not undergo IFS splitting; but better safe than sorry. (But note that
> 'export foo=$bar', which is not POSIX, is *not* an assignment, and
> different shells that support this construct treat it differently.)

Since Junio asked(?) that it be removed, I'll drop it from the next rev
of the patchset.

quoted text
> > +### Test that we handle strange characters properly
> > +work_dir="$(pwd)/test \" ' $ \ dir"
> 
> In effect, you modify only this test to stress-test strange characters,
> but other tests in the test suite still run in a "sane" environment. IOW,
> I don't think you should go to this extreme for this one test only. The
> better approach would be to rename 'trash' in test-lib.sh to this strange
> name so that all tests suffer from a challenging environment.

I can do that, but it'd have to come as the last patch in the patchset,
or it would obviously cause test failures. My goal here was to ensure
that the bug I fixed in the patch would be tested in an isolated manner.

If you like I can add a change to the trash directory to the next rev of
the patchset.

quoted text
> 
> > -		git reset --hard pre-rebase
> > -		test_must_fail git rebase'"$type"' master &&
> > -		test -d '$dotest' &&
> > +		git reset --hard pre-rebase &&
> > +		test_must_fail git rebase$type master &&
> > +		test -d \"$dotest\" &&
> 
> I could imagine that the missing && after the git reset is deliberate. Mike?

I assumed that if git reset failed here we'd probably want to know :)

Thanks,

Bryan
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html