Re: [PATCH v7 2/2] ident: add user.useConfigOnly boolean for when ident shouldn't be guessed

[Dan Aloni <alonid@gmail.com>]

On Fri, Feb 05, 2016 at 04:48:33PM -0500, Jeff King wrote:
> On Fri, Feb 05, 2016 at 11:29:06PM +0200, Dan Aloni wrote:
> 
> > diff --git a/t/t9904-per-repo-email.sh b/t/t9904-per-repo-email.sh
> > new file mode 100755
> > index 000000000000..f2b33881e46b
> > --- /dev/null
> > +++ b/t/t9904-per-repo-email.sh
> 
> Is t9904 the right place for this? Usually t99xx is for very separate
> components.
> 
> This is sort-of about "commit", which would put it in the t75xx range.
> But in some ways, it is even more fundamental than that. We don't seem
> to have a lot of tests for ident stuff. The closest is the strict ident
> stuff in t0007.

Will move to t7517. IMHO it's better to verify the commit operation
itself before running further tests that rely on its proper function.

>[..]
> > +reprepare () {
> > +	git reset --hard initial
> > +}
> 
> Do we need this reprepare stuff at all now? The tests don't care which
> commit we're at when they start.
> 
> > +test_expect_success setup '
> > +	# Initial repo state
> > +	echo "Initial" >foo &&
> > +	git add foo &&
> > +	git commit -m foo &&
> > +	git tag initial &&
> 
> A shorter way of saying this is "test_commit foo".
> 
> I almost thought we could get rid of this part entirely; the commit
> tests don't care. But we do still need _a_ commit for the clone test,
> since we want to make sure a reflog is written. It would be nice to push
> it down there, but our test environment doesn't allow creating commits,
> because of of useConfigOnly. So it's probably fine to leave it here.
> 
> Technically, the final "commit" test does make a commit for us to push,
> but we do generally try to avoid unnecessary dependencies between the
> individual tests.
> 
> So all together, maybe:
>[..]

Yes, shorted is better.

I'm squashing in these changes and adding you as Signed-off for v8.

-- 
Dan Aloni