[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: FYI: do-release-commit-and-tag: fix typo
From: |
Jim Meyering |
Subject: |
Re: FYI: do-release-commit-and-tag: fix typo |
Date: |
Thu, 05 Jul 2012 16:53:30 +0200 |
Akim Demaille wrote:
> Le 5 juil. 2012 à 15:24, Akim Demaille a écrit :
>
>> Hi friends,
>>
>> I just installed this.
>>
>> # simple check: no question marks on line 3 of NEWS
>> -test "$(sed -n 3p NEWS)" = "$noteworthy_stub" \
>> +test "$(sed -n 3p NEWS)" != "$noteworthy_stub" \
>> || die 'line 3 of NEWS looks fishy!'
>
> This was wrong, the = was right, it is the comments that
> are misleading. I'm trying to make a beta of Bison, and
> because an initial run changed the NEWS file (which I did
> not notice) the NEWS line was changed, and this check
> was triggered. I thought that this check was ensuring
> that the template was properly filled, but actually,
> it's exactly the opposite: this is run _before_.
>
> I'm sorry for noise. I propose the following patch
> which is first performing the checks, to avoid useless
> changes of NEWS, and comment/message fixes.
>
> Also, meanwhile Jim answered:
>
>>> # simple check: no question marks on line 3 of NEWS
>>> -test "$(sed -n 3p NEWS)" = "$noteworthy_stub" \
>>> +test "$(sed -n 3p NEWS)" != "$noteworthy_stub" \
>>> || die 'line 3 of NEWS looks fishy!'
>>
>> First, that's sort of like a double negative.
>> This is a more readable equivalent of what you've committed:
>>
>> test "$(sed -n 3p NEWS)" = "$noteworthy_stub" \
>> && die 'line 3 of NEWS looks fishy!'
>
> Well, while I agree in general, I don't have the same
> feeling in this case, for two reasons. The first one
> being that under set -e, && propagate the failure,
> and the script will die. || is safer than && when
This script doesn't use set -e.
I'd say that having to limit such use of "&&" is a good reason
not to use "set -e".
> not in a condition (inside an if for instance). And second,
> this pattern (check || die) implements assert is a quite
> visual way. Using &&, on the contrary IMHO, would make
> things harder to read (because not consistent).
>
> Anyway, this was wrong in this place.
>
> Here is my proposal:
>
> From 6df4446131184e583ebc2c472a626fed07aab012 Mon Sep 17 00:00:00 2001
> From: Akim Demaille <address@hidden>
> Date: Thu, 5 Jul 2012 15:41:20 +0200
> Subject: [PATCH] do-release-commit-and-tag: fix the previous commit
>
> * build-aux/do-release-commit-and-tag: Actually the test was right,
> but the error message was wrong.
As far as I can tell, the message is not wrong.
> Fix comment, and improve error message.
This is accurate.
> Perform check first, so that NEWS is not modified uselessly.
> ---
> ChangeLog | 6 ++++++
> build-aux/do-release-commit-and-tag | 41
> ++++++++++++++++++++++++-------------
> 2 files changed, 33 insertions(+), 14 deletions(-)
>
> diff --git a/ChangeLog b/ChangeLog
> index c257cb2..57478ed 100644
> --- a/ChangeLog
> +++ b/ChangeLog
> @@ -1,5 +1,11 @@
> 2012-07-05 Akim Demaille <address@hidden>
>
> + do-release-commit-and-tag: fix the previous commit
> + * build-aux/do-release-commit-and-tag: Actually the test was right,
> + but the error message was wrong.
> + Fix comment, and improve error message.
> + Perform check first, so that NEWS is not modified uselessly.
> +
> do-release-commit-and-tag: fix typo
> * build-aux/do-release-commit-and-tag: Be sure that NEWS does
> _not_ start with a stub.
> diff --git a/build-aux/do-release-commit-and-tag
> b/build-aux/do-release-commit-and-tag
> index 329d0eb..78223be 100755
> --- a/build-aux/do-release-commit-and-tag
> +++ b/build-aux/do-release-commit-and-tag
> @@ -3,7 +3,7 @@
> # controlled .prev-version file, automate the procedure by which we record
> # the date, release-type and version string in the NEWS file. That commit
> # will serve to identify the release, so apply a signed tag to it as well.
> -VERSION=2012-07-05.13 # UTC
> +VERSION=2012-07-05.14 # UTC
>
> # Note: this is a bash script (could be zsh or dash)
>
> @@ -77,6 +77,10 @@ EOF
> exit
> }
>
> +## ------ ##
> +## Main. ##
> +## ------ ##
> +
> branch=master
> builddir=.
>
> @@ -107,6 +111,11 @@ test $# = 2 \
> ver=$1
> type=$2
>
> +
> +## ---------------------- ##
> +## First, sanity checks. ##
> +## ---------------------- ##
> +
> # Verify that $ver looks like a version number, and...
> echo "$ver"|grep -E '^[0-9][0-9.]*[0-9]$' > /dev/null \
> || die "invalid version: $ver"
> @@ -124,32 +133,36 @@ case $type in
> *) die "invalid release type: $type";;
> esac
>
> +# No dirt allowed.
s/dirt/local modifications/
> +case $(git diff-index --name-only HEAD) in
> + '') ;;
> + *) die 'this tree is dirty; commit your changes first';;
> +esac
> +
> +# Ensure the current branch name is correct:
> +curr_br=$(git rev-parse --symbolic-full-name HEAD)
> +test "$curr_br" = refs/heads/$branch || die not on branch $branch
> +
> # Extract package name from Makefile.
> Makefile=$builddir/Makefile
> pkg=$(sed -n 's/^PACKAGE = \(.*\)/\1/p' "$Makefile") \
> || die "failed to determine package name from $Makefile"
>
> -# simple check: no question marks on line 3 of NEWS
> -test "$(sed -n 3p NEWS)" != "$noteworthy_stub" \
> - || die 'line 3 of NEWS looks fishy!'
> +# Check that line 3 of NEWS is the stub line about to be filled.
s/filled/replaced/ (or "with blanks about to be filled in")
ACK with those changes.
- FYI: do-release-commit-and-tag: fix typo, Akim Demaille, 2012/07/05
- Re: FYI: do-release-commit-and-tag: fix typo, Jim Meyering, 2012/07/05
- Re: FYI: do-release-commit-and-tag: fix typo, Akim Demaille, 2012/07/05
- Re: FYI: do-release-commit-and-tag: fix typo,
Jim Meyering <=
- Re: FYI: do-release-commit-and-tag: fix typo, Akim Demaille, 2012/07/05
- Re: FYI: do-release-commit-and-tag: fix typo, Eric Blake, 2012/07/05
- Re: FYI: do-release-commit-and-tag: fix typo, Akim Demaille, 2012/07/06