| Message ID | 20191009194740.8079-2-crosa@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | iotests: trivial cleanups | expand |
----- Original Message ----- > From: "Eric Blake" <eblake@redhat.com> > To: "Cleber Rosa" <crosa@redhat.com>, qemu-devel@nongnu.org > Cc: qemu-block@nongnu.org, "Max Reitz" <mreitz@redhat.com>, "Kevin Wolf" <kwolf@redhat.com>, qemu-trivial@nongnu.org, > "Michael Tokarev" <mjt@tls.msk.ru>, "Laurent Vivier" <laurent@vivier.eu> > Sent: Wednesday, October 9, 2019 3:51:56 PM > Subject: Re: [PATCH v2 1/4] qemu-iotests: remove bash shebang from library files > > On 10/9/19 2:47 PM, Cleber Rosa wrote: > > Due to not being able to find a reason to have shebangs on files that > > are not executable. > > > > While at it, add a mode hint to emacs, which would be clueless or > > plain wrong about these containing shell code. > > > > Suggested-by: Eric Blake <eblake@redhat.com> > > Signed-off-by: Cleber Rosa <crosa@redhat.com> > > --- > > tests/qemu-iotests/common.config | 2 +- > > tests/qemu-iotests/common.filter | 2 +- > > tests/qemu-iotests/common.nbd | 3 +-- > > tests/qemu-iotests/common.pattern | 2 +- > > tests/qemu-iotests/common.qemu | 2 +- > > tests/qemu-iotests/common.rc | 2 +- > > tests/qemu-iotests/common.tls | 2 +- > > 7 files changed, 7 insertions(+), 8 deletions(-) > > > > diff --git a/tests/qemu-iotests/common.config > > b/tests/qemu-iotests/common.config > > index 9bd1a5a6fc..b85a6a6f96 100644 > > --- a/tests/qemu-iotests/common.config > > +++ b/tests/qemu-iotests/common.config > > @@ -1,4 +1,4 @@ > > -#!/usr/bin/env bash > > +# -*- emacs mode: sh -*- > > I thought my version: > # hey emacs, this file will be sourced by bash -*- mode: sh -*- > was cuter, but that's not a requirement, and yours works ;) > I will not make any disputes on that! :) Thanks for the reviews! - Cleber. > Reviewed-by: Eric Blake <eblake@redhat.com> > > -- > Eric Blake, Principal Software Engineer > Red Hat, Inc. +1-919-301-3226 > Virtualization: qemu.org | libvirt.org >
Am 09.10.2019 um 21:47 hat Cleber Rosa geschrieben: > Due to not being able to find a reason to have shebangs on files that > are not executable. > > While at it, add a mode hint to emacs, which would be clueless or > plain wrong about these containing shell code. vim still doesn't like the change. Of course, we could also add another line for vim and for every other editor in use, but actually, I think I'd prefer just dropping this patch. It even makes each file a few bytes larger instead of saving something. Shebang lines are a shorter and more portable format indicator than the alternatives. So I think in the end we have found a good reason to keep them. :-) Kevin
On Fri, Oct 11, 2019, 12:36 Kevin Wolf <kwolf@redhat.com> wrote: > Am 09.10.2019 um 21:47 hat Cleber Rosa geschrieben: > > Due to not being able to find a reason to have shebangs on files that > > are not executable. > > > > While at it, add a mode hint to emacs, which would be clueless or > > plain wrong about these containing shell code. > > vim still doesn't like the change. > > Of course, we could also add another line for vim and for every other > editor in use, but actually, I think I'd prefer just dropping this > patch. It even makes each file a few bytes larger instead of saving > something. Shebang lines are a shorter and more portable format > indicator than the alternatives. > > So I think in the end we have found a good reason to keep them. :-) > What about .sh suffix? Should be most portable way. >
On Fri, Oct 11, 2019 at 02:27:25PM +0300, Nir Soffer wrote: > On Fri, Oct 11, 2019, 12:36 Kevin Wolf <kwolf@redhat.com> wrote: > > > Am 09.10.2019 um 21:47 hat Cleber Rosa geschrieben: > > > Due to not being able to find a reason to have shebangs on files that > > > are not executable. > > > > > > While at it, add a mode hint to emacs, which would be clueless or > > > plain wrong about these containing shell code. > > > > vim still doesn't like the change. > > > > Of course, we could also add another line for vim and for every other > > editor in use, but actually, I think I'd prefer just dropping this > > patch. It even makes each file a few bytes larger instead of saving > > something. Shebang lines are a shorter and more portable format > > indicator than the alternatives. > > > > So I think in the end we have found a good reason to keep them. :-) > > > > What about .sh suffix? Should be most portable way. > > > That's the approach I tend to follow for my sh code. Explicit is better than implicit if you ask me. Kevin, Do you have any strong feelings here? I'd be fine with either this or dropping the patch. Thanks, - Cleber.
Am 11.10.2019 um 22:05 hat Cleber Rosa geschrieben: > On Fri, Oct 11, 2019 at 02:27:25PM +0300, Nir Soffer wrote: > > On Fri, Oct 11, 2019, 12:36 Kevin Wolf <kwolf@redhat.com> wrote: > > > > > Am 09.10.2019 um 21:47 hat Cleber Rosa geschrieben: > > > > Due to not being able to find a reason to have shebangs on files that > > > > are not executable. > > > > > > > > While at it, add a mode hint to emacs, which would be clueless or > > > > plain wrong about these containing shell code. > > > > > > vim still doesn't like the change. > > > > > > Of course, we could also add another line for vim and for every other > > > editor in use, but actually, I think I'd prefer just dropping this > > > patch. It even makes each file a few bytes larger instead of saving > > > something. Shebang lines are a shorter and more portable format > > > indicator than the alternatives. > > > > > > So I think in the end we have found a good reason to keep them. :-) > > > > What about .sh suffix? Should be most portable way. > > That's the approach I tend to follow for my sh code. Explicit is > better than implicit if you ask me. I would certainly agree for new files. > Kevin, > > Do you have any strong feelings here? I'd be fine with either this > or dropping the patch. No strong feelings. The result of renaming the files would be a bit nicer than what we have today, but renaming always comes with a cost when working with the version history later. Hard to tell if it's a net gain or loss in the end. Myself, I would probably pick the lazy way and stick with "if it ain't broke, don't fix it", but I'm not objecting to a change either. Kevin
diff --git a/tests/qemu-iotests/common.config b/tests/qemu-iotests/common.config index 9bd1a5a6fc..b85a6a6f96 100644 --- a/tests/qemu-iotests/common.config +++ b/tests/qemu-iotests/common.config @@ -1,4 +1,4 @@ -#!/usr/bin/env bash +# -*- emacs mode: sh -*- # # Copyright (C) 2009 Red Hat, Inc. # Copyright (c) 2000-2003,2006 Silicon Graphics, Inc. All Rights Reserved. diff --git a/tests/qemu-iotests/common.filter b/tests/qemu-iotests/common.filter index 9f418b4881..a0b52b0ac8 100644 --- a/tests/qemu-iotests/common.filter +++ b/tests/qemu-iotests/common.filter @@ -1,4 +1,4 @@ -#!/usr/bin/env bash +# -*- emacs mode: sh -*- # # Copyright (C) 2009 Red Hat, Inc. # Copyright (c) 2000-2001 Silicon Graphics, Inc. All Rights Reserved. diff --git a/tests/qemu-iotests/common.nbd b/tests/qemu-iotests/common.nbd index 24b01b60aa..a98d05d404 100644 --- a/tests/qemu-iotests/common.nbd +++ b/tests/qemu-iotests/common.nbd @@ -1,5 +1,4 @@ -#!/usr/bin/env bash -# -*- shell-script-mode -*- +# -*- emacs mode: sh -*- # # Helpers for NBD server related config # diff --git a/tests/qemu-iotests/common.pattern b/tests/qemu-iotests/common.pattern index 4f5e5bcea0..4028b32c54 100644 --- a/tests/qemu-iotests/common.pattern +++ b/tests/qemu-iotests/common.pattern @@ -1,4 +1,4 @@ -#!/usr/bin/env bash +# -*- emacs mode: sh -*- # # Copyright (C) 2009 Red Hat, Inc. # diff --git a/tests/qemu-iotests/common.qemu b/tests/qemu-iotests/common.qemu index 8d2021a7eb..66569f6bdd 100644 --- a/tests/qemu-iotests/common.qemu +++ b/tests/qemu-iotests/common.qemu @@ -1,4 +1,4 @@ -#!/usr/bin/env bash +# -*- emacs mode: sh -*- # # This allows for launching of multiple QEMU instances, with independent # communication possible to each instance. diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc index e45cdfa66b..aa4a7fcc11 100644 --- a/tests/qemu-iotests/common.rc +++ b/tests/qemu-iotests/common.rc @@ -1,4 +1,4 @@ -#!/usr/bin/env bash +# -*- emacs mode: sh -*- # # Copyright (C) 2009 Red Hat, Inc. # Copyright (c) 2000-2006 Silicon Graphics, Inc. All Rights Reserved. diff --git a/tests/qemu-iotests/common.tls b/tests/qemu-iotests/common.tls index 54c331d7a5..759a3d6d71 100644 --- a/tests/qemu-iotests/common.tls +++ b/tests/qemu-iotests/common.tls @@ -1,4 +1,4 @@ -#!/usr/bin/env bash +# -*- emacs mode: sh -*- # # Helpers for TLS related config #
Due to not being able to find a reason to have shebangs on files that are not executable. While at it, add a mode hint to emacs, which would be clueless or plain wrong about these containing shell code. Suggested-by: Eric Blake <eblake@redhat.com> Signed-off-by: Cleber Rosa <crosa@redhat.com> --- tests/qemu-iotests/common.config | 2 +- tests/qemu-iotests/common.filter | 2 +- tests/qemu-iotests/common.nbd | 3 +-- tests/qemu-iotests/common.pattern | 2 +- tests/qemu-iotests/common.qemu | 2 +- tests/qemu-iotests/common.rc | 2 +- tests/qemu-iotests/common.tls | 2 +- 7 files changed, 7 insertions(+), 8 deletions(-)