[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