[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] maint: edit-readme-alpha shouldn't try to re-edit during dis
From: |
Gary V. Vaughan |
Subject: |
Re: [PATCH] maint: edit-readme-alpha shouldn't try to re-edit during dist. |
Date: |
Tue, 21 Sep 2010 13:44:41 +0700 |
On 21 Sep 2010, at 12:07, Ralf Wildenhues wrote:
> Hi Gary,
Hallo Ralf,
Thanks again for the review.
> * Gary V. Vaughan wrote on Tue, Sep 21, 2010 at 03:05:46AM CEST:
>> Well, it does at least show that the script interacts correctly with
>> an error for make to help catch the case where someone commits a change
>> to the first paragraph of README without a matching edit to the sed
>> expressions in edit-readme-alpha.
>>
>> Here is (an overkill) patch to not exit with an error, so that `make
>> distcheck' can complete.
>>
>> Okay to push?
>
> Well, I don't want to appear overly nitpicky for this issue,
> but if you don't absolutely need to check and skip non-writable files,
> I wouldn't do it, and rather error out in that case.
Well, the thing that is screwing up distcheck is when the distribution
tarball is unpacked into a read-only tree, and edit-readme-alpha wants
to rewrite the file again...
...although, the "don't re-edit when the alpha text is already there"
check later on should catch this already. Although I feel happier with
both checks in place, I'll remove the writeable test if you'd prefer.
>> * libltdl/config/edit-readme-alpha: If README is non-writable
>> assume that it is being run from distcheck, and bail out with
>> a warning
>
> FWIW, exiting 0 is not what I'd call "bail out".
I mean "bail out" as in: don't do the rest of the processing.
>> for file in "$@"; do
>> + # Assume that read-only README indicates that we are running inside
>> + # the latter part of a `make distcheck'.
>> + test -w $file || {
>> + echo "$progname: not editing non-writeable \`$file' (distcheck?)"
>
> If you prefer to, or need to keep the warning+skip, I suggest to
> print warnings on stderr.
Oops. Well caught, thanks.
>> + continue
>> + }
>> +
>> # Make sure the paragraph we are matching has not been edited since
>> # this script was written.
>> - matched=`sed -n -e '/^This is GNU Libtool,/,/^interface.$/p' $file \
>> + matched=`sed -n -e '/^This is GNU Libtool,/,/^interface\.$/p' $file \
>> |wc -l |sed 's|^ *||'`
>> +
>> + # Unless, of course, it was edited by this script already.
>> + test 3 = "$matched" \
>> + || matched=`sed -n -e '/^This is an alpha testing release/,/behind a
>> consistent, portable interface\.$/p' $file \
>> + |wc -l |sed 's|^ *||'`
>
> Indentation is a bit weird here, I'd have expected the | to align one
> after the ` in the line above.
Okay. I was lining it up with previous clause, but it is ugly. I'll fix
that too.
With those addressed, and another successful distcheck, okay to push?
Cheers,
--
Gary V. Vaughan (address@hidden)
PGP.sig
Description: This is a digitally signed message part
- Re: [PATCH 2/6] maint: consolidate Introductions of README and README.alpha., (continued)
- [PATCH 1/6] maint: copy the Version Numbering section into README.alpha., Gary V. Vaughan, 2010/09/18
- [PATCH 4/6] maint: reformat README `The Test Suites' for consistency., Gary V. Vaughan, 2010/09/18
- [PATCH 6/6] maint: use sed instead of maintaining 2 README files., Gary V. Vaughan, 2010/09/18
- Re: [PATCH 6/6] maint: use sed instead of maintaining 2 README files., Ralf Wildenhues, 2010/09/18
- Re: [PATCH 6/6] maint: use sed instead of maintaining 2 README files., Gary V. Vaughan, 2010/09/18
- Re: [PATCH 6/6] maint: use sed instead of maintaining 2 README files., Ralf Wildenhues, 2010/09/20
- [PATCH] maint: edit-readme-alpha shouldn't try to re-edit during dist., Gary V. Vaughan, 2010/09/20
- Re: [PATCH] maint: edit-readme-alpha shouldn't try to re-edit during dist., Ralf Wildenhues, 2010/09/21
- Re: [PATCH] maint: edit-readme-alpha shouldn't try to re-edit during dist.,
Gary V. Vaughan <=
- Re: [PATCH] maint: edit-readme-alpha shouldn't try to re-edit during dist., Ralf Wildenhues, 2010/09/21
Re: README cleanups, Ralf Wildenhues, 2010/09/18