voxpupuli / puppet-nginx

Puppet Module to manage NGINX on various UNIXes
https://forge.puppet.com/puppet/nginx
MIT License
471 stars 882 forks source link

Becoming #PuppetApproved #465

Closed jfryman closed 5 years ago

jfryman commented 10 years ago

The folks at PuppetLabs just released a set of guidelines to become #PuppetApproved. This issue will track progress toward that goal.

Full spec can be found here: https://forge.puppetlabs.com/approved/criteria

Style

PuppetLint.configuration.fail_on_warnings
PuppetLint.configuration.send('relative')
PuppetLint.configuration.send('disable_80chars')
PuppetLint.configuration.send('disable_class_inherits_from_params_class')
PuppetLint.configuration.send('disable_class_parameter_defaults')
PuppetLint.configuration.send('disable_documentation')
PuppetLint.configuration.send('disable_single_quote_string_with_variables')
PuppetLint.configuration.ignore_paths = ["spec//*.pp", "pkg//*.pp"]

Documentation

3flex commented 10 years ago

For the "Style" section I'm assuming those settings don't have to be in Rakefile. Our .puppet-lint.rc already has:

--fail-on-warnings
--no-80chars-check
--no-class_inherits_from_params_class-check

so it's already more strict than those guidelines. The only missing one from that section is the second last item.

jfryman commented 10 years ago

@3flex Excellent! Let's check those off!

3flex commented 10 years ago

Well I spoke too soon, puppetlabs_spec_helper is broken so we actually weren't checking style violations. Waiting on https://github.com/puppetlabs/puppetlabs_spec_helper/pull/78 for a fix but for now I have pinned to 0.8.0. Because there are some violations the build's now broken.

Also now it's actually running again I see a violation of the documentation check, so that actually should be disabled in our config per the guidelines. I've unchecked "added to CI tests" and "disable-documentation" from the checklist to reflect the current situation.

danieldreier commented 10 years ago

I'm not 100% sure, but I don't believe that modules depending upon module_data will be puppet approved.

Monkey patching core puppet effectively invalidates the acceptance testing suite. RI describes it as "... consider this a early feedback-saught release" in the readme, and there's no test coverage, so I'm not at all confident it's production ready.

I do really like the functionality module_data provides; I'm just skeptical that untested monkey patches will fly in module approval.

jfryman commented 10 years ago

@danieldreier yes, this is understood. I've already had some back-channel talks with the PuppetLabs team about puppet-module-data, and some of the ramifications of it. Haven't come to a clear decision, but this is a topic we're discussing.

3flex commented 9 years ago

@jfryman I've refreshed the list and updated with current status.

There are two remaining "musts":

Let me know if you want assistance with anything. I will refresh #451 as well and comment there when I think you can reconsider merging.

I suggest opening a new ticket with a checklist for anything that should be done before 1.x.

wyardley commented 8 years ago

Docs is still a mess. @bastelfreak: do we want to leave this open until we improve the docs? Are we concerned about being #puppetapproved?

anarcat commented 5 years ago

we are puppet approved now, so i'm just going to close this. there are separate issues (#1245, #451) for improving docs.