[augeas-devel] Prototype of xml lens

David Lutterkort lutter at redhat.com
Sat Jul 24 00:34:45 UTC 2010


Hi Francis,

On Tue, 2010-07-20 at 22:06 -0400, Francis Giraldeau wrote:
> For those that may want to try the XML lens, here it is. The latest
> square2 branch is needed. Lot of work to be done, but still it shows
> what it's possible to do. 

I just had a look at your branch, and it looks very good. There's some
minor nits that I noticed on a quick look over the code:

      * for development, you should run autogen.sh with
        --enable-compile-warnings=error - this will make sure that gcc
        barfs on as many things as possible. Attached is the trivial
        patch to fix failures from a couple bad printf's
      * stylistically, try to match the style of the existing code as
        much as possible (e.g., in 'if (cond) {' there's always a space
        before the '{')
      * Can we rename state->tip to something more obvious, like
        state->square ?

We should figure out how to best get your changes into the tree, so I
can review them fully. I suggest that your square2 branch should become
3 patch series, which you can send in independently of each other. The
first one should be about adding the square lens, the second adding the
XML lens, and the third the httpd lens.

You should restructure your patches in such a way that each of these
patch series can stand on their own; the XML and httpd lens can each be
a single patch. For the square lens, a single patch might be enough, too
(unless you can think of an obvious way to split that up so that it's
easier to review; one strict requirement is that 'make check' must pass
after each individual patch has been applied) I hope you're familiar
with 'git rebase -i' - if not, you'll really like it by the time this is
all done.

cheers,
David
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-fix-compile-error.patch
Type: text/x-patch
Size: 1193 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/augeas-devel/attachments/20100723/95d5ee69/attachment.bin>


More information about the augeas-devel mailing list