[PATCH/puppet 1/1] Fix #2784 - puppetdoc/rdoc didn't parse mono-instruction class content

5 views
Skip to first unread message

Brice Figureau

unread,
Nov 4, 2009, 2:28:28 PM11/4/09
to puppe...@googlegroups.com
class klass {
include a, b, c
}

wasn't producing any rdoc documentation.
We were thinking code was always embedded in an array which is not
the case for mono-instruction code.

Signed-off-by: Brice Figureau <brice-...@daysofwonder.com>
---
lib/puppet/util/rdoc/parser.rb | 3 +++
spec/unit/util/rdoc/parser.rb | 18 ++++++++++++++++++
2 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/lib/puppet/util/rdoc/parser.rb b/lib/puppet/util/rdoc/parser.rb
index aa34335..d7e1c30 100644
--- a/lib/puppet/util/rdoc/parser.rb
+++ b/lib/puppet/util/rdoc/parser.rb
@@ -148,6 +148,7 @@ class Parser
# create documentation for include statements we can find in +code+
# and associate it with +container+
def scan_for_include_or_require(container, code)
+ code = [code] unless code.is_a?(Array)
code.each do |stmt|
scan_for_include_or_require(container,stmt.children) if stmt.is_a?(Puppet::Parser::AST::ASTArray)

@@ -163,6 +164,7 @@ class Parser
# create documentation for global variables assignements we can find in +code+
# and associate it with +container+
def scan_for_vardef(container, code)
+ code = [code] unless code.is_a?(Array)
code.each do |stmt|
scan_for_vardef(container,stmt.children) if stmt.is_a?(Puppet::Parser::AST::ASTArray)

@@ -176,6 +178,7 @@ class Parser
# create documentation for resources we can find in +code+
# and associate it with +container+
def scan_for_resource(container, code)
+ code = [code] unless code.is_a?(Array)
code.each do |stmt|
scan_for_resource(container,stmt.children) if stmt.is_a?(Puppet::Parser::AST::ASTArray)

diff --git a/spec/unit/util/rdoc/parser.rb b/spec/unit/util/rdoc/parser.rb
index b05df6d..de11832 100755
--- a/spec/unit/util/rdoc/parser.rb
+++ b/spec/unit/util/rdoc/parser.rb
@@ -321,6 +321,12 @@ describe RDoc::Parser do
@code.stubs(:is_a?).with(Puppet::Parser::AST::ASTArray).returns(true)
end

+ it "should also scan mono-instruction code" do
+ @class.expects(:add_include).with { |i| i.is_a?(RDoc::Include) and i.name == "myclass" and i.comment == "mydoc" }
+
+ @parser.scan_for_include_or_require(@class,create_stmt("include"))
+ end
+
it "should register recursively includes to the current container" do
@code.stubs(:children).returns([ create_stmt("include") ])

@@ -354,6 +360,12 @@ describe RDoc::Parser do
@class.expects(:add_constant).with { |i| i.is_a?(RDoc::Constant) and i.name == "myvar" and i.comment == "mydoc" }
@parser.scan_for_vardef(@class, [ @code ])
end
+
+ it "should also scan mono-instruction code" do
+ @class.expects(:add_constant).with { |i| i.is_a?(RDoc::Constant) and i.name == "myvar" and i.comment == "mydoc" }
+
+ @parser.scan_for_vardef(@class, @stmt)
+ end
end

describe "when scanning for resources" do
@@ -375,6 +387,12 @@ describe RDoc::Parser do
@class.expects(:add_resource).with { |i| i.is_a?(RDoc::PuppetResource) and i.title == "myfile" and i.comment == "mydoc" }
@parser.scan_for_resource(@class, [ @code ])
end
+
+ it "should also scan mono-instruction code" do
+ @class.expects(:add_resource).with { |i| i.is_a?(RDoc::PuppetResource) and i.title == "myfile" and i.comment == "mydoc" }
+
+ @parser.scan_for_resource(@class, @stmt)
+ end
end

describe "when parsing plugins" do
--
1.6.4

Brice Figureau

unread,
Nov 4, 2009, 2:35:56 PM11/4/09
to puppe...@googlegroups.com
Note: I pushed 2 branches, one for 0.25.x and one for master because it
touches code that diverged between 0.25.x and master, but the fix is the
same (except master has some tests).


--
Brice Figureau
My Blog: http://www.masterzen.fr/

Markus Roberts

unread,
Nov 4, 2009, 2:57:01 PM11/4/09
to puppe...@googlegroups.com
Note: I pushed 2 branches, one for 0.25.x and one for master because it
touches code that diverged between 0.25.x and master, but the fix is the
same (except master has some tests)

So call me an old fuddy, but a lot of these sorts of problems don't happen when you just apply patches rather than merging branches.

Brice Figureau

unread,
Nov 4, 2009, 4:43:48 PM11/4/09
to puppe...@googlegroups.com

I suspect you'd get exactly the same issues with patches in the case I'm
referring to.

I fail to see how the same patch could apply to two diverged codebases.
Sure you'll get a conflict, exactly what the merge will produce (indeed
merging is not more than applying patches, or am I wrong?).

I practiced too much quilt and other manual patch stacks for my mental
health, I'm happy with VCS and especially git :-)

James Turnbull

unread,
Nov 4, 2009, 4:49:47 PM11/4/09
to puppe...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Brice Figureau wrote:

> I practiced too much quilt and other manual patch stacks for my mental
> health, I'm happy with VCS and especially git :-)

I second that. Divergence can happen for lots of reasons - one of
the most basic is "we'll fix this like this in 0.25.x and properly
in master".

Regards

James Turnbull

- --
Author of:
* Pro Linux System Administration (http://tinyurl.com/linuxadmin)
* Pulling Strings with Puppet (http://tinyurl.com/pupbook)
* Pro Nagios 2.0 (http://tinyurl.com/pronagios)
* Hardening Linux (http://tinyurl.com/hardeninglinux)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSvH2+iFa/lDkFHAyAQLjBAgAlP1NZ8Jz/tlupMmdphpKtZoYONk2JnSD
QNUgScKEPDi84rMAxr7viGxPZslz8LxTA1t86q/qUm9KDoPg/UTh121q2YFeBp9o
AvwRr6Y2LSIVZcm6xPHAuQ2te+AIIG4jftfneQ+SBG4ibB3qq1cNfHeOMnWFfYJE
V17Ql2QZ0uT9Z3CbvubOGa8P0ZjcQDE8Zju7/QRO5/4AJnaBIu48uTWQBc7Mtrh/
cpFBWIPo+Y55SprySs1TGR0tNKEcNp1z0WXsVAbjd5Gckiy6CuLKygsExJNESQof
UtCNrDC+i9oboHGjd5ScKL0sCPrZXMaOKJdLWwLdHNU86wm3DkN94w==
=MBWG
-----END PGP SIGNATURE-----

Markus Roberts

unread,
Nov 4, 2009, 4:54:02 PM11/4/09
to puppe...@googlegroups.com

*smile*  Yeah, as I said, I'm most likely just being an old fuddy (We used to apply our patches in the snow.  It was uphill both ways.  And we liked it.  Etc.)

-- Markus


James Turnbull

unread,
Nov 4, 2009, 5:04:54 PM11/4/09
to puppe...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

You forgot "we only had half a rotten doughnut to eat". :)

Regards

James Turnbull

- --
Author of:
* Pro Linux System Administration (http://tinyurl.com/linuxadmin)
* Pulling Strings with Puppet (http://tinyurl.com/pupbook)
* Pro Nagios 2.0 (http://tinyurl.com/pronagios)
* Hardening Linux (http://tinyurl.com/hardeninglinux)
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSvH6hiFa/lDkFHAyAQKQOAf/Y2Uei/sbDft8OeOCsKkXTB2qdECZTVq4
/caDvvSgs7AJXtulDKlfoMmBeKuYeO64BX09JtDxoXPzE3Nd2hXIfPQNj65KC0vE
G2K/ubSbpORVBEKOOnuv2dfyUaJ8POuFvnreIzFp9gGjo/YSp57BQ8Zeb21RaIpS
/X5JRDxNx3oQa6ReLTLYr7wJHbr6EnfvvLfYMrgoVqb+BzL9HTqO7H6XPVzhcFPQ
TxdrqJ7gftoudgNRrIKBda4QOb88QDU4kld6mCxUQtKeglxUc82Uy9+TVCjAfzTt
5/2KM6hYvbF6L9b6naIDxYE2jJZJRG8hCiruwhA7tCbidYkWUi08ug==
=+C8A
-----END PGP SIGNATURE-----

Luke Kanies

unread,
Nov 5, 2009, 3:19:37 PM11/5/09
to puppe...@googlegroups.com

How would one actually do that, without using mutt and mboxes?

AFAICT, none of the "just apply patches" solutions allow you to use
mail clients created since 1990.

I've considered it in the past, but it never seemed feasible without
requiring everyone to revert two decades.

--
When one admits that nothing is certain one must, I think, also admit
that some things are much more nearly certain than others.
-- Bertrand Russell
---------------------------------------------------------------------
Luke Kanies | http://reductivelabs.com | http://madstop.com

Brice Figureau

unread,
Nov 5, 2009, 4:32:32 PM11/5/09
to puppe...@googlegroups.com
On 05/11/09 21:19, Luke Kanies wrote:
>
> On Nov 4, 2009, at 1:57 PM, Markus Roberts wrote:
>
>>
>>
>> Note: I pushed 2 branches, one for 0.25.x and one for master because
>> it
>> touches code that diverged between 0.25.x and master, but the fix is
>> the
>> same (except master has some tests)
>>
>> So call me an old fuddy, but a lot of these sorts of problems don't
>> happen when you just apply patches rather than merging branches.
>
> How would one actually do that, without using mutt and mboxes?
>
> AFAICT, none of the "just apply patches" solutions allow you to use
> mail clients created since 1990.
>
> I've considered it in the past, but it never seemed feasible without
> requiring everyone to revert two decades.

I think Evolution could work quite fine, because it doesn't wrap
patches. If only Evolution was working on OSX.
And actually mutt is pretty powerful :-)

Luke Kanies

unread,
Nov 5, 2009, 4:52:11 PM11/5/09
to puppe...@googlegroups.com

I agree mutt is powerful, I just don't want to use it. And really,
it's more the server-side technology: You have to use local mboxes
instead of remote IMAP mailboxes. What we really need is an imap-
based patch management solution. But I don't think IMAP has much in
the way of ordering, so I don't know if that would even suffice.

--
Think twice before you speak, and then you may be able to say
something more insulting than if you spoke right out at
once. -- Evan Esar

Todd Zullinger

unread,
Nov 5, 2009, 5:26:51 PM11/5/09
to puppe...@googlegroups.com
Luke Kanies wrote:
> I agree mutt is powerful, I just don't want to use it. And really,
> it's more the server-side technology: You have to use local mboxes
> instead of remote IMAP mailboxes. What we really need is an imap-
> based patch management solution. But I don't think IMAP has much in
> the way of ordering, so I don't know if that would even suffice.

Anyone have experience with patchwork¹? It looks like a nice tool,
but I've never taken the time to set it up and actually use it.

¹ http://ozlabs.org/~jk/projects/patchwork/

--
Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
I'm not concerned about all hell breaking loose, but that a PART of
hell will break loose... it'll be much harder to detect.
-- George Carlin

James Turnbull

unread,
Nov 5, 2009, 6:01:43 PM11/5/09
to puppe...@googlegroups.com
2009/11/6 Todd Zullinger <t...@pobox.com>:
> Luke Kanies wrote:
> ¹ http://ozlabs.org/~jk/projects/patchwork/
>

We did a lot of looking about 6 months ago at a bunch of tools - code
review, patch management, etc. I looked at Patchwork too because
Jeremy Kerr and the OzLabs guys are people I know. It was good but
had some issues and ordering and tracking groups of patches. In fact
that was the key issue with most of the tools - they couldn't parse
chains of patches and they didn't have good methods to handle patch
evolution - i.e. create patch, comment on patch, patch updated,
comment cycle, update patch, approve. Each of these transactions
generally created a new "review" request rather than evolving the
original request.

Regards

James Turnbull

Ramon van Alteren

unread,
Nov 6, 2009, 3:17:45 AM11/6/09
to puppe...@googlegroups.com
Hi,

On Thu, Nov 05, 2009 at 03:52:11PM -0600, Luke Kanies wrote:
> On Nov 5, 2009, at 3:32 PM, Brice Figureau wrote:
> > I think Evolution could work quite fine, because it doesn't wrap
> > patches. If only Evolution was working on OSX.
> > And actually mutt is pretty powerful :-)
>
> I agree mutt is powerful, I just don't want to use it. And really,
> it's more the server-side technology: You have to use local mboxes
> instead of remote IMAP mailboxes.

I just switched to mutt, and one of the requirements was that it should
do IMAP, 'cause that's the only mailboxes I have.

I have it setup to operate directly on my IMAP mailboxes, including
Exchange and gmail and that is working just fine for me.

Did you guys look at the just released tool gerrit for patch/review
management ? From the release it looks like it's focus is on reviewing
patches rather than managing sets of them. But it might be usable ?

Regards,

Ramon
--

Teamlead System Engineering Hyves.nl

e: ra...@hyves.nl
w: http://ramon71.hyves.nl

Frederiksplein 42
1017 XN Amsterdam
t +31 20 624 20 81
f +31 20 750 83 29

Eric Gerlach

unread,
Nov 6, 2009, 11:44:09 AM11/6/09
to puppe...@googlegroups.com
On Thu, Nov 05, 2009 at 03:52:11PM -0600, Luke Kanies wrote:
> I agree mutt is powerful, I just don't want to use it. And really,
> it's more the server-side technology: You have to use local mboxes
> instead of remote IMAP mailboxes. What we really need is an imap-
> based patch management solution. But I don't think IMAP has much in
> the way of ordering, so I don't know if that would even suffice.

I'm using mutt+imap just fine. What I would to to manage git patches from that
is tag all the messages you want to apply, then save them to a local file.
That creates your mbox to pull into git.

Caveat: I could be talking out of my ass, because I haven't done much patch
applying from mail, mostly just emailing patches.

I don't know if that helps, but hopefully it will help someone. I'm always
learning more mutt-fu.

Cheers,

--
Eric Gerlach, Network Administrator
Federation of Students
University of Waterloo
p: (519) 888-4567 x36329
e: eger...@feds.uwaterloo.ca

Todd Zullinger

unread,
Nov 6, 2009, 11:49:23 AM11/6/09
to puppe...@googlegroups.com
Eric Gerlach wrote:
> I'm using mutt+imap just fine. What I would to to manage git
> patches from that is tag all the messages you want to apply, then
> save them to a local file. That creates your mbox to pull into git.
>
> Caveat: I could be talking out of my ass, because I haven't done
> much patch applying from mail, mostly just emailing patches.

That does work, but only if the sender's mail client (or some MTA or
mailing list software along the way) doesn't wrap or otherwise
butcher the mail. This is hard to achieve with so many different mail
clients in use. :/

--
Todd OpenPGP -> KeyID: 0xBEAF0CE3 | URL: www.pobox.com/~tmz/pgp
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

It's better to be wanted for murder that not to be wanted at all.
-- Marty Winch

James Turnbull

unread,
Nov 6, 2009, 4:14:06 PM11/6/09
to puppe...@googlegroups.com
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Ramon van Alteren wrote:
> Did you guys look at the just released tool gerrit for patch/review
> management ? From the release it looks like it's focus is on reviewing
> patches rather than managing sets of them. But it might be usable ?

I looked at Gerrit too again had similar issues around linking
groups of patches together and the evolution of patches.

Regards

James Turnbull

- --


Author of:
* Pro Linux System Administration (http://tinyurl.com/linuxadmin)
* Pulling Strings with Puppet (http://tinyurl.com/pupbook)
* Pro Nagios 2.0 (http://tinyurl.com/pronagios)
* Hardening Linux (http://tinyurl.com/hardeninglinux)

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.7 (Darwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iQEVAwUBSvSRniFa/lDkFHAyAQKbbQgAtFyAH6vJI9tkHVQyEqiRntL6NCXLvMZG
ADWX33irTUcucnOWtYvrluKBj0Nq+Q2tpE/TOwuc5GV/8wzetdtc0vFC1zLkLFwj
pygS2vGV1BeJIbQARzynWt3Mf6CKE03hxvGB+RMNBSxWA/dHNd4x1g7vW9Ovkvy3
h3lEl1qYtDB0FcACcwXN5ofx1ghclJMnJYde/pksKMCMRYrMSbazWkWBuRWqw0l8
st2fchYSz4VB1xEHLaStWEafVeVEx+KeuNBwftEwmMmPTbNbYtfdhmwKca6xW5is
F0baXgKjaG4FxO3J+KtUyEBN3T8zduKb6Mzkoh56kwRbNTkMYcwxrA==
=ZlWK
-----END PGP SIGNATURE-----

Luke Kanies

unread,
Nov 6, 2009, 8:50:43 PM11/6/09
to puppe...@googlegroups.com
On Nov 6, 2009, at 10:44 AM, Eric Gerlach wrote:

>
> On Thu, Nov 05, 2009 at 03:52:11PM -0600, Luke Kanies wrote:
>> I agree mutt is powerful, I just don't want to use it. And really,
>> it's more the server-side technology: You have to use local mboxes
>> instead of remote IMAP mailboxes. What we really need is an imap-
>> based patch management solution. But I don't think IMAP has much in
>> the way of ordering, so I don't know if that would even suffice.
>
> I'm using mutt+imap just fine. What I would to to manage git
> patches from that
> is tag all the messages you want to apply, then save them to a local
> file.
> That creates your mbox to pull into git.

Yeah, the imap part is easy - it's the mbox part. I know it's
possible, it's just... not so fun.

> Caveat: I could be talking out of my ass, because I haven't done
> much patch
> applying from mail, mostly just emailing patches.
>
> I don't know if that helps, but hopefully it will help someone. I'm
> always
> learning more mutt-fu.

The need to learn mutt-fu is why I stopped using it. :)

--
A Chemical Limerick:
A mosquito cried out in pain:
"A chemist has poisoned my brain!"
The cause of his sorrow
was para-dichloro
diphenyltrichloroethane
-- Dr. D. D. Perrin

Luke Kanies

unread,
Nov 6, 2009, 8:51:24 PM11/6/09
to puppe...@googlegroups.com
On Nov 6, 2009, at 2:17 AM, Ramon van Alteren wrote:

> Hi,
>
> On Thu, Nov 05, 2009 at 03:52:11PM -0600, Luke Kanies wrote:
>> On Nov 5, 2009, at 3:32 PM, Brice Figureau wrote:
>>> I think Evolution could work quite fine, because it doesn't wrap
>>> patches. If only Evolution was working on OSX.
>>> And actually mutt is pretty powerful :-)
>>
>> I agree mutt is powerful, I just don't want to use it. And really,
>> it's more the server-side technology: You have to use local mboxes
>> instead of remote IMAP mailboxes.
>
> I just switched to mutt, and one of the requirements was that it
> should
> do IMAP, 'cause that's the only mailboxes I have.
>
> I have it setup to operate directly on my IMAP mailboxes, including
> Exchange and gmail and that is working just fine for me.
>
> Did you guys look at the just released tool gerrit for patch/review
> management ? From the release it looks like it's focus is on reviewing
> patches rather than managing sets of them. But it might be usable ?

I think we looked at it for code review, but we've never really looked
at patch management apps beyond the basic review.

--
The Internet, of course, is more than just a place to find pictures of
people having sex with dogs. -- Time Magazine, 3 July 1995

Markus Roberts

unread,
Nov 10, 2009, 3:19:02 PM11/10/09
to puppe...@googlegroups.com
+1

As a by-the-way, you could also write this:


    +        code = [code] unless code.is_a?(Array)
              code.each do |stmt|

as:

    -         code.each do |stmt|
    +        [code].flatten.each do |stmt|

And finally, I'm very sorry for dropping the stray remark that hijacked the thread.  T'wasn't my intent.

-- Markus

Brice Figureau

unread,
Nov 10, 2009, 5:04:27 PM11/10/09
to puppe...@googlegroups.com
On 10/11/09 21:19, Markus Roberts wrote:
> +1
>
> As a by-the-way, you could also write this:
>
> + code = [code] unless code.is_a?(Array)
> code.each do |stmt|
>
> as:
>
> - code.each do |stmt|
> + [code].flatten.each do |stmt|

I longly hesitated but finally preferred the first version because
flatten acts recursively on the array which might not be what I wanted
(honestly I don't really remember).

> And finally, I'm very sorry for dropping the stray remark that hijacked the
> thread. T'wasn't my intent.

Absolutely no problem, it's just nobody cared anymore about my patch, so
I felt it was necessary to resurrect it :-D

Reply all
Reply to author
Forward
0 new messages