From: Todd Zullinger Date: Fri, 01 Dec 2017 03:40:47 GMT Subject: Re: [PATCH 0/2] t/lib-git-svn.sh: improve svnserve tests with parallel make test Message-ID: <20171201034047.GC3693@zaya.teonanacatl.net> In-Reply-To: <20171201030727.GH20640@aiede.mtv.corp.google.com> Hi Jonathan, Jonathan Nieder wrote: > Todd Zullinger wrote: > >> These tests are not run by default nor are they enabled in travis-ci. I >> don't know how much testing they get in user or other packager builds. >> >> I've been slowly increasing the test suite usage in fedora builds. I >> ran into this while testing locally with parallel make test. The >> official fedora builds don't run in parallel (yet), as even before I ran >> into this issue, builds on the fedora builders randomly failed too >> often. I'm hoping to eventually enable parallel tests by default >> though, since it's so much faster. > > This background could go in the commit message for patch 2, but it > also speaks for itself as an obviously good change so I could go > either way. Heh. If there's something in there in particular that seems useful, I can certainly add it. I'm not sure what parts of this text would be beneficial to someone down the line though. I usually err on the 'too much information' side of commit messages. I'm happy that it's much harder to do that here. I'd rather have to skim a long message than wonder about the motivation for a change. >> I'm not sure if there's any objection to changing the variable needed to >> enable the tests from SVNSERVE_PORT to GIT_TEST_SVNSERVE. The way >> SVNSERVE_PORT is set in this patch should allow the port to be set >> explicitly, in case someone requires that -- and they understand that it >> can fail if running parallel tests, of course. Whether that's a >> feature or a bug, I'm not sure. :) > > micronit: can this just say something like > > Patch 2 is the important one --- see that one for rationale. > > Patch 1 is an optional preparatory style cleanup. > > next time? That way, you get an automatic guarantee that all the > important information is available in "git log" output to people who > need it later. Yeah, I'll try to remember that. I started this without the whitespace cleanup and as I was writing in the single patch description that I didn't think the whitespace cleanup was warranted, I talked myself into doing it as the prep patch. :) Thanks, -- Todd ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ Tradition: Just because you've always done it that way doesn't mean it's not incredibly stupid. -- Demotivators (www.despair.com)