Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

(PDK-388, PDK-392) Add README, CHANGELOG, and puppet requirement to module generation #232

Merged
merged 4 commits into from
Aug 11, 2017

Conversation

bmjen
Copy link
Contributor

@bmjen bmjen commented Aug 8, 2017

-Adds a puppet requirement to metadata.json
-Adds generating a skeleton README.md when generating new module
-Adds generating a skeleton CHANGELOG.md when generating new module

Requires https://github.com/puppetlabs/pdk-module-template/pull/24

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.9%) to 91.296% when pulling 0a97e76 on bmjen:misc-new-module into f8cb71d on puppetlabs:master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.9%) to 91.296% when pulling 120517c on bmjen:misc-new-module into f8cb71d on puppetlabs:master.

@bmjen bmjen changed the title New module generation improvements. (PDK-388, PDK-392) New module generation improvements. Aug 9, 2017
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 92.105% when pulling b7119f2 on bmjen:misc-new-module into f8cb71d on puppetlabs:master.

@bmjen
Copy link
Contributor Author

bmjen commented Aug 9, 2017

Acceptance tests will fail until https://github.com/puppetlabs/pdk-module-template/pull/24 is merged.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 92.105% when pulling 2ec0a9d on bmjen:misc-new-module into f8cb71d on puppetlabs:master.

@@ -34,7 +34,7 @@ class TemplateDir
# @raise [ArgumentError] (see #validate_module_template!)
#
# @api public
def initialize(path_or_url)
def initialize(path_or_url, userdata = {})
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we change this variable name to module_metadata to be a bit more descriptive?

Choose a reason for hiding this comment

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

Also needs a yardoc entry

@@ -221,6 +223,7 @@ def config_for(dest_path)
if File.file?(config_path) && File.readable?(config_path)
begin
@config = YAML.safe_load(File.read(config_path), [], [], true)
@config.merge!('global' => { 'userdata' => @userdata })
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than merging this into the @config hash (which i'm planning to eventually deprecate), it would be cleaner (imho) to add this as the module_metadata key in the file_config hash, and then access it in the templates as module_metadata['version'] etc.

@@ -27,5 +27,15 @@
'operatingsystemrelease' => ['8']))
end
end

describe file('foo/README.md') do

Choose a reason for hiding this comment

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

These new examples are dependent on the module template providing these files. I think it's more valuable to have these examples, with the risk of pdk-module-template breaking them; than to not have them 👍

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 92.105% when pulling 419f82d on bmjen:misc-new-module into f8cb71d on puppetlabs:master.

(fixup) Fixes unit tests for new module generator
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.1%) to 92.105% when pulling 44786f9 on bmjen:misc-new-module into f8cb71d on puppetlabs:master.

@bmjen
Copy link
Contributor Author

bmjen commented Aug 9, 2017

New updates require: https://github.com/puppetlabs/pdk-module-template/pull/27 to pass tests.

@james-stocks
Copy link

Coveralls informs how many new lines of code are causing a coverage drop (in this case, 2 lines in lib/pdk/module/templatedir.rb ) : https://coveralls.io/builds/12769685

@bmjen we should at least have unit tests that satisfy coveralls.
If there's a need for higher levels of testing for generated content let me know. Possibly we need to address pdk cloning pdk-module-template master, to remove the tests being dependent on the state of that repo?

@bmjen
Copy link
Contributor Author

bmjen commented Aug 10, 2017

@james-stocks There is a ticket open to address defaulting to using the templates in the package. This should improve the pdk->pdk-module-template dependency a bit, however we do need to enact some structure on the pdk-module-template to aid in making sure compatibility isn't a problem. We'll need to start versioning the pdk-module-template repo and add version ranges in pdk to prevent breakage.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.1%) to 93.333% when pulling 665e3f1 on bmjen:misc-new-module into f8cb71d on puppetlabs:master.

@bmjen bmjen merged commit 3b2265d into puppetlabs:master Aug 11, 2017
@bmjen bmjen deleted the misc-new-module branch August 11, 2017 05:31
@DavidS DavidS changed the title (PDK-388, PDK-392) New module generation improvements. (PDK-388, PDK-392) Add README, CHANGELOG, and puppet requirement to module generation Aug 11, 2017
@DavidS DavidS added the feature label Aug 11, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants