[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v6 1/2] bitmaps.md: Convert to rST; move it into
From: |
Kashyap Chamarthy |
Subject: |
Re: [Qemu-block] [PATCH v6 1/2] bitmaps.md: Convert to rST; move it into 'interop' dir |
Date: |
Tue, 11 Jul 2017 09:58:39 +0200 |
User-agent: |
Mutt/1.6.0.1 (2016-04-01) |
On Mon, Jul 10, 2017 at 09:36:50PM -0500, Eric Blake wrote:
> On 07/10/2017 03:15 AM, Kashyap Chamarthy wrote:
[...]
> > Signed-off-by: Kashyap Chamarthy <address@hidden>
> > Reviewed-by: John Snow <address@hidden>
> > ---
>
> > ---
> > docs/devel/bitmaps.md | 505 ------------------------------------------
> > docs/interop/bitmaps.rst | 555
> > +++++++++++++++++++++++++++++++++++++++++++++++
>
> A shame that git rename detection doesn't see these as the same rough
> contents, but not too bad.
_Should_ it detect?
> I'll just review the new text; if I point out
> something that was pre-existing in the old text, it may be nicer to
> split the cleanups into a separate followup patch, but I'm also okay if
> they go in as part of this patch.
Indeed -- the things you point out further below were already
pre-existing.
Asking out of curiosity: You say it is nicer split because we'll retain
the `git-bisect`-ability? Or just to keep text motion and actual
changes separate? (I think both.)
> My review is less focused on the choices used in representing things in
> rST and on the final display outcome, and more on the text itself
> (grammar and such).
Noted.
>
> > +++ b/docs/interop/bitmaps.rst
> > +
> > +- Dirty bitmaps can be created at any time and attached to any node
> > + (not just complete drives.)
>
> Looks a bit nicer with '.' outside the sub-sentence '()'.
Will fix.
> > +
> > +.. contents::
> > +
> > +Dirty Bitmap Names
> > +------------------
> > +
> > +- A dirty bitmap's name is unique to the node, but bitmaps attached to
> > + different nodes can share the same name.
> > +
> > +- Dirty bitmaps created for internal use by QEMU may be anonymous and
> > + have no name, but any user-created bitmaps may not be. There can be
>
> s/may not be/must have a name/
Will fix.
>
> > +
> > +- This bitmap will have a default granularity that matches the cluster
> > + size of its associated drive, if available, clamped to between [4KiB,
>
> s/clamped to/clamped/
Will fix.
> > + 64KiB]. The current default for qcow2 is 64KiB.
> > +
> > +
>
>
> > +- QMP example response, highlighting one success and one failure:
> > +
> > + - Acknowledgement that the Transaction was accepted and jobs were
> > + launched:
>
> US spelling prefers Acknowledgment, but I think you are okay because of
> UK spelling.
It was John Snow who wrote this document. :-)
I realize that you merely pointed out the difference. And the QEMU
upstream documentation also says they accept both US and UK spellings:
http://wiki.qemu.org/Contribute/SpellCheck
> Again, since these are probably pre-existing and just code motion, and
> if you've gotten positive review of your use of rST, I'm fine with adding:
>
> Reviewed-by: Eric Blake <address@hidden>
Thank you.
I'll wait for feedback on the other (much longer and completely
overhauled) document -- if there's a need to respin it, I'll address
your feedback from here.
[...]
--
/kashyap
[Qemu-block] [PATCH v6 2/2] live-block-ops.txt: Rename, rewrite, and improve it, Kashyap Chamarthy, 2017/07/10