Skip to content

Add support for enabling puppetdb report processor#64

Merged
kbarber merged 5 commits into
puppetlabs:masterfrom
cprice404:feature/master/support-enabling-report-processor
May 28, 2013
Merged

Add support for enabling puppetdb report processor#64
kbarber merged 5 commits into
puppetlabs:masterfrom
cprice404:feature/master/support-enabling-report-processor

Conversation

@cprice404

Copy link
Copy Markdown

No description provided.

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build triggered. (Status: PENDING, Details: null)

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build started. (Status: PENDING, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/51/)

Comment thread manifests/master/config.pp Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like an accidental whitespace change.

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build finished. (Status: SUCCESS, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/51/)

@cprice404

Copy link
Copy Markdown
Author

@nicklewis yeah I think there was a tab in there that was showing up weird in my editor. I fixed all of those lines to use regular spaces and squashed a commit. Thanks

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build triggered. (Status: PENDING, Details: null)

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build started. (Status: PENDING, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/53/)

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build finished. (Status: SUCCESS, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/53/)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We keep copying this non-standard puppet doc header ... this is the style we should follow: http://docs.puppetlabs.com/guides/style_guide.html#puppet-doc but we probably need to do this everywhere as well. Its just that running this kind of header through rdoc makes it look terrible. Not a big deal of course, just thought I'd mention it.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kbarber is it the missing equals signs that you're referring to?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No I mean the entire comment for the class.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, but I'm only seeing minor differences between what you linked to and what's in this code. Namely: equals signs for headers, asterisks around parameter names... that's pretty much it?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cprice-puppet yeah thats about right, names of header sections as well are different (for example 'actions' could just be in the main description, and its 'examples' instead of sample usage). The parameters section renders differently using the format in the style guide also. While it looks like small differences, the difference in rendering makes it more presentable when put through the 'puppet doc' tool. Its not a big deal like I said, I'm happy to do a cleanup of all of this later if you like - getting all the classes, since I'm the one being pedantic :-).

@kbarber

kbarber commented May 24, 2013

Copy link
Copy Markdown
Contributor

This looks good, although we need:

  • To update README.md, since this is what the forge displays
  • Add a basic rspec-puppet test, nothing too complicated.

Comment thread spec/system/basic_spec.rb

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You didn't close the 'let' here.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aha! thx. kind of surprised that didn't throw an error.

Add docs to readme, add basic rspec-puppet tests for new
report_processor class, and fix broken rspec-system test.
@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build triggered. (Status: PENDING, Details: null)

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build triggered. (Status: PENDING, Details: null)

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build finished. (Status: FAILURE, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/55/)

@kbarber

kbarber commented May 24, 2013

Copy link
Copy Markdown
Contributor

retest this please

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build triggered. (Status: PENDING, Details: null)

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build started. (Status: PENDING, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/57/)

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build finished. (Status: FAILURE, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/57/)

@cprice404

Copy link
Copy Markdown
Author

retest this please

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build triggered. (Status: PENDING, Details: null)

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build started. (Status: PENDING, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/58/)

@kbarber-jenkins-bot

Copy link
Copy Markdown

Merged build finished. (Status: SUCCESS, Details: http://box.bob.sh:8080/job/puppetlabs-puppetdb-system/58/)

kbarber added a commit that referenced this pull request May 28, 2013
…ling-report-processor

Add support for enabling puppetdb report processor
@kbarber kbarber merged commit b45fca9 into puppetlabs:master May 28, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants