From mboxrd@z Thu Jan  1 00:00:00 1970
From: George Dunlap <george.dunlap@eu.citrix.com>
Subject: Re: [PATCH 1/5] raisin: introduce _verbose_echo
Date: Tue, 21 Apr 2015 16:09:08 +0100
Message-ID: <55366814.2000700@eu.citrix.com>
References: <alpine.DEB.2.02.1504211551510.3112@kaball.uk.xensource.com>
	<1429628103-10979-1-git-send-email-stefano.stabellini@eu.citrix.com>
Mime-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
Return-path: <xen-devel-bounces@lists.xen.org>
In-Reply-To: <1429628103-10979-1-git-send-email-stefano.stabellini@eu.citrix.com>
List-Unsubscribe: <http://lists.xen.org/cgi-bin/mailman/options/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=unsubscribe>
List-Post: <mailto:xen-devel@lists.xen.org>
List-Help: <mailto:xen-devel-request@lists.xen.org?subject=help>
List-Subscribe: <http://lists.xen.org/cgi-bin/mailman/listinfo/xen-devel>,
	<mailto:xen-devel-request@lists.xen.org?subject=subscribe>
Sender: xen-devel-bounces@lists.xen.org
Errors-To: xen-devel-bounces@lists.xen.org
To: Stefano Stabellini <stefano.stabellini@eu.citrix.com>, xen-devel@lists.xensource.com
Cc: george.dunlap@citrix.com
List-Id: xen-devel@lists.xenproject.org

On 04/21/2015 03:54 PM, Stefano Stabellini wrote:
> A new utility function to make the code more readable and compact:
> prints a message if VERBOSE = 1.
> 
> Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>
> ---
>  lib/common-functions.sh |   46 ++++++++++++++++------------------------------
>  1 file changed, 16 insertions(+), 30 deletions(-)
> 
> diff --git a/lib/common-functions.sh b/lib/common-functions.sh
> index dc3a2bb..c19046b 100644
> --- a/lib/common-functions.sh
> +++ b/lib/common-functions.sh
> @@ -1,5 +1,12 @@
>  #!/usr/bin/env bash
>  
> +function _verbose_echo() {
> +    if [[ $VERBOSE -eq 1 ]]
> +    then
> +        echo $*
> +    fi
> +}

Why _?  Don't we have VERBOSE in other parts of the code that could call
this?

 -George

> +
>  # Executed once at the beginning of the script
>  function common_init() {
>      export BASEDIR=`pwd`
> @@ -33,12 +40,9 @@ function common_init() {
>      get_arch
>      get_components
>  
> -    if [[ $VERBOSE -eq 1 ]]
> -    then
> -        echo "Distro: $DISTRO"
> -        echo "Arch: $ARCH"
> -        echo "Components: $COMPONENTS"
> -    fi
> +    _verbose_echo "Distro: $DISTRO"
> +    _verbose_echo "Arch: $ARCH"
> +    _verbose_echo "Components: $COMPONENTS"
>  
>      for f in $COMPONENTS
>      do
> @@ -62,10 +66,7 @@ function get_components() {
>              if eval [[ ! -z \$"$capital"_REVISION ]]
>              then
>                  COMPONENTS="$COMPONENTS $component"
> -                if [[ $VERBOSE -eq 1 ]]
> -                then
> -                    echo "Found component $component"
> -                fi
> +                _verbose_echo "Found component $component"
>              fi
>          done
>      fi
> @@ -166,10 +167,7 @@ function get_arch() {
>  }
>  
>  function _check-package-deb() {
> -    if [[ $VERBOSE -eq 1 ]]
> -    then
> -        echo "Checking for package ${args[0]}"
> -    fi
> +    _verbose_echo "Checking for package ${args[0]}"
>  
>      if dpkg -s "$1" 2>/dev/null | grep -q "Status:.*installed"
>      then
> @@ -184,10 +182,7 @@ function _install-package-deb() {
>  }
>  
>  function _check-package-rpm() {
> -    if [[ $VERBOSE -eq 1 ]]
> -    then
> -        echo "Checking for package $1"
> -    fi
> +    _verbose_echo "Checking for package $1"
>  
>      if rpm -q "$1" 2>&1 >/dev/null
>      then
> @@ -273,22 +268,13 @@ function for_each_component () {
>          done
>          if [[ $found -eq 0 ]]
>          then
> -            if [[ $VERBOSE -eq 1 ]]
> -            then
> -                echo "$component" is disabled
> -            fi
> +            _verbose_echo "$component" is disabled
>              continue
>          fi
>  
> -        if [[ $VERBOSE -eq 1 ]]
> -        then
> -            echo calling "$component"_"$1"
> -        fi
> +        _verbose_echo calling "$component"_"$1"
>          "$component"_"$1"
> -        if [[ $VERBOSE -eq 1 ]]
> -        then
> -            echo "$component"_"$1" done
> -        fi
> +        _verbose_echo "$component"_"$1" done
>      done
>  }
>  
>