On Mon, Nov 20, 2017 at 5:44 AM, Alex Bennée <alex.bennee@xxxxxxxxxx> wrote: > Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes: >> It is not at all clear, based upon this text, what this is fixing. >> When you re-roll, please provide a description of the regression in >> sufficient detail for readers to easily understand the problem and the >> solution. > > How about: > > Since the removal of Mail::Address from git-send-email certain addresses > common in MAINTAINERS now fail to get correctly parsed by > Git::parse_mailboxes. Specifically the patterns with embedded > parenthesis fail, for example for MAINTAINERS: > [...snip...] Thanks, that explanation makes the problem quite clear. It also allowed me to examine the fix with a more critical eye, which led to several additional comments and observations (sent in my previous email). >> Use write_script() to create the script: > > Thanks for the pointers, I'll fix it up. > > I missed the existence of write_script.sh while I scanned through > t/README, perhaps a stanza in in "Test harness library": > > - write_script <name> <<-\EOF && <rest of test> > echo '...' > ... > EOF > > The write_script helper takes care of ensuring the created helper > script has the right shebang and is executable. > ? Mentioning write_script() in the "Test harness library" section might indeed be a good idea. > Ahh that makes sense. Again perhaps in the t/README "Keep in mind:" > > - All the tests in a given test file run sequentially and share > repository state. This means you should take care not to break > assumptions of later tests as to which commits exist in the test > repository. > ? Maybe, maybe not. Ideally, tests should be as self-contained as possible. In practice, of course, this isn't always practical or even possible (and there is a lot of old test code which is heavily dependent upon state left over from earlier tests). Perfectly self-contained tests (or self-contained _sets_ of tests) could theoretically be run in parallel, so, in general, we'd like to encourage people to tend toward self-contained, thus the above snippet may be going in the wrong direction.
