Libguestfs - Aug 2018 - Re: [PATCH] v2v: rhv-plugin: Use string literal concatenation

On Mon, Aug 6, 2018 at 11:02 AM Richard W.M. Jones <rjones@redhat.com>
wrote:
> On Sun, Aug 05, 2018 at 04:37:31PM +0300, Nir Soffer wrote:
> > When splitting long strings over multiple lines, we can use string
> > literal concatenation instead of +.
> >
> > See
>
https://docs.python.org/3/reference/lexical_analysis.html#string-literal-concatenation
> > ---
> >  .gnulib                  |  2 +-
>
> I dropped the accidental gnulib part of this commit :-)
>
Sorry about that :-)

I think I did:
git pull
git checkout -b ...
edit file
git commit
git format-patch master

Can we prevent submodules changes to sneak into unrelated patches
without manual work on the developer side?

>
> Rich.
>
> >  v2v/rhv-upload-plugin.py | 14 +++++++-------
> >  2 files changed, 8 insertions(+), 8 deletions(-)
> >
> > diff --git a/.gnulib b/.gnulib
> > index 5b78831df..646a44e1b 160000
> > --- a/.gnulib
> > +++ b/.gnulib
> > @@ -1 +1 @@
> > -Subproject commit 5b78831df03b49408676227604cf16f90dee07ac
> > +Subproject commit 646a44e1b190c4a7f6a9f32c63230c619e38d251
> > diff --git a/v2v/rhv-upload-plugin.py b/v2v/rhv-upload-plugin.py
> > index bdc1e104a..2d686c2da 100644
> > --- a/v2v/rhv-upload-plugin.py
> > +++ b/v2v/rhv-upload-plugin.py
> > @@ -95,8 +95,8 @@ def find_host(connection):
> >          # - 'hw_id' equals to 'vdsm_id'
> >          # - Its status is 'Up'
> >          # - Belongs to the storage domain's datacenter
> > -        debug("cannot find a running host with hw_id=%r, "
+
> > -              "that belongs to datacenter '%s', " +
> > +        debug("cannot find a running host with hw_id=%r, "
> > +              "that belongs to datacenter '%s', "
> >                "using any host" % (vdsm_id,
datacenter.name))
> >          return None
> >
> > @@ -193,15 +193,15 @@ def open(readonly):
> >          if transfer.phase != types.ImageTransferPhase.INITIALIZING:
> >              break
> >          if time.time() > endt:
> > -            raise RuntimeError("timed out waiting for transfer
status "
> +
> > +            raise RuntimeError("timed out waiting for transfer
status "
> >                                 "!= INITIALIZING")
> >
> >      # Now we have permission to start the transfer.
> >      if params['rhv_direct']:
> >          if transfer.transfer_url is None:
> > -            raise RuntimeError("direct upload to host not
supported, " +
> > -                               "requires ovirt-engine >= 4.2
and only
> works " +
> > -                               "when virt-v2v is run within the
> oVirt/RHV " +
> > +            raise RuntimeError("direct upload to host not
supported, "
> > +                               "requires ovirt-engine >= 4.2
and only
> works "
> > +                               "when virt-v2v is run within the
> oVirt/RHV "
> >                                 "environment, eg. on an oVirt
node.")
> >          destination_url = urlparse(transfer.transfer_url)
> >      else:
> > @@ -511,7 +511,7 @@ def close(h):
> >                  time.sleep(1)
> >                  tmp = transfer_service.get()
> >                  if time.time() > endt:
> > -                    raise RuntimeError("timed out waiting for
transfer
> " +
> > +                    raise RuntimeError("timed out waiting for
transfer "
> >                                         "to finalize")
> >          except sdk.NotFoundError:
> >              pass
> > --
> > 2.17.1
>
> --
> Richard Jones, Virtualization Group, Red Hat
> http://people.redhat.com/~rjones
> Read my programming and virtualization blog: http://rwmj.wordpress.com
> virt-builder quickly builds VMs from scratch
> http://libguestfs.org/virt-builder.1.html
>

On Mon, Aug 06, 2018 at 12:11:31PM +0300, Nir Soffer
wrote:
> On Mon, Aug 6, 2018 at 11:02 AM Richard W.M. Jones
<rjones@redhat.com>
> wrote:
> 
> > On Sun, Aug 05, 2018 at 04:37:31PM +0300, Nir Soffer wrote:
> > > When splitting long strings over multiple lines, we can use
string
> > > literal concatenation instead of +.
> > >
> > > See
> >
https://docs.python.org/3/reference/lexical_analysis.html#string-literal-concatenation
> > > ---
> > >  .gnulib                  |  2 +-
> >
> > I dropped the accidental gnulib part of this commit :-)
> >
> 
> Sorry about that :-)
> 
> I think I did:
> git pull
> git checkout -b ...
> edit file
> git commit
I wonder if you used ‘git commit -a’ which would have added all
changes from the local directory, and thus included ‘.gnulib’.  Anyway
it doesn't really matter as it was no problem for me to remove the
errant change.
> git format-patch master
I have a wrapper around git-send-email so I can just do:

  send-email " nbdkit v2" HEAD^ libguestfs@redhat.com [CC ...]

(see attached - NB it needs some customization before use).
> Can we prevent submodules changes to sneak into unrelated patches
> without manual work on the developer side?
I believe they won't be added unless you explicitly added them to the
commit, or use ‘git commit -a’.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top

--8cpS+6Cx+xtICsjy
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment; filename=send-email

#!/bin/sh -
# Sane wrapper around git send-email.
#
# Example:
#   send-email " v2" HEAD~10 libguestfs@redhat.com CC@example.com
CC@bar.example.com
# To test, send it to yourself first:
#   send-email " v2" HEAD~10 rjones@redhat.com

set -e

smtp=smtp.corp.redhat.com

if [ $# -lt 3 ]; then
    echo "send-email: Read the script first."
    exit 1
fi

subject_prefix="--subject-prefix=PATCH$1"
shift

# Arguments.
range="$1"
shift

to="$1"
shift

cc=""
for x in "$@"; do
    cc="$cc --cc=$x"
done

git send-email \
    --compose \
    --stat \
    --thread --no-chain-reply-to \
    --envelope-sender="rjones@redhat.com" \
    --smtp-server=$smtp \
    "$subject_prefix" \
    --from="Richard W.M. Jones <rjones@redhat.com>" \
    --to="$to" $cc \
    "$range"

--8cpS+6Cx+xtICsjy--

On Mon, Aug 6, 2018 at 1:27 PM Richard W.M. Jones <rjones@redhat.com>
wrote:
> On Mon, Aug 06, 2018 at 12:11:31PM +0300, Nir Soffer wrote:
> > On Mon, Aug 6, 2018 at 11:02 AM Richard W.M. Jones
<rjones@redhat.com>
> > wrote:
> >
> > > On Sun, Aug 05, 2018 at 04:37:31PM +0300, Nir Soffer wrote:
> > > > When splitting long strings over multiple lines, we can use
string
> > > > literal concatenation instead of +.
> > > >
> > > > See
> > >
>
https://docs.python.org/3/reference/lexical_analysis.html#string-literal-concatenation
> > > > ---
> > > >  .gnulib                  |  2 +-
> > >
> > > I dropped the accidental gnulib part of this commit :-)
> > >
> >
> > Sorry about that :-)
> >
> > I think I did:
> > git pull
> > git checkout -b ...
> > edit file
> > git commit
>
> I wonder if you used ‘git commit -a’ which would have added all
> changes from the local directory, and thus included ‘.gnulib’.

I never use "git commit -a" for this reason. I used "make"
and "git add
-u", and
it did pick up the changes in the submodule. Maybe we need to ignore
these?