Pkg xen devel - Dec 2020 - [PATCH 02/19] d/shuffle-binaries: Remove useless extra argument being passed in

On 9/19/20 8:24 AM, Elliott Mitchell wrote:
> This doesn't do anything.  Really it is almost a bug the script
/doesn't/
> complain.
Hah, yes, good catch. The second argument is never used inside
shuffle-binaries.

In shuffle-binaries, I see "version=$1; shift". Ian, is the shift and
the not-used but passed $(flavour) a left-over from some intermediate
steps while doing the WHRAAA reworking of the packaging?

I guess we could add dropping that "; shift" in this same change,
because it only confuses the reader into thinking that there will be
some code later on using $1 again?
> Signed-off-by: Elliott Mitchell <ehem+debian at m5p.com>
> ---
>  debian/rules | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/debian/rules b/debian/rules
> index 741676e617..c7dc55ad03 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -271,7 +271,7 @@ xenstore_rm = $(addprefix debian/xen-utils-common/,		\
>  		))
>  
>  override_dh_install:
> -	debian/shuffle-binaries $(upstream_version) $(flavour)
> +	debian/shuffle-binaries $(upstream_version)
>  	:
>  	debian/shuffle-boot-files $(upstream_version) $(flavour)
>  	:
> 
Thanks,
Hans

On Fri, Dec 04, 2020 at 09:14:32PM +0100, Hans van Kranenburg
wrote:
> On 9/19/20 8:24 AM, Elliott Mitchell wrote:
> > This doesn't do anything.  Really it is almost a bug the script
/doesn't/
> > complain.
> 
> Hah, yes, good catch. The second argument is never used inside
> shuffle-binaries.
> 
> In shuffle-binaries, I see "version=$1; shift". Ian, is the shift
and
> the not-used but passed $(flavour) a left-over from some intermediate
> steps while doing the WHRAAA reworking of the packaging?
> 
> I guess we could add dropping that "; shift" in this same change,
> because it only confuses the reader into thinking that there will be
> some code later on using $1 again?
The reason to *not* do that seems pretty obvious.  Having your argument
taking lines all follow the form "<var>=$1; shift" increases
consistency.
Other theory is this leaves the environment cleaner.  With the shift,
only $0 is left valid and $# is left zero.

You'll notice in shuffle-binaries:
version="$1"; shift

In shuffle-boot-files before the change:
version="$1"; shift
flavour="$1"; shift

The shift survived simply for consistency/documentation sake.  $(flavour)
might have been left in due to historic usage, but it also looks like it
may have originated as a copy and paste of
`debian/shuffle-boot-files $(upstream_version) $(flavour)`


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg at m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445