-
Notifications
You must be signed in to change notification settings - Fork 105
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
(SDK-200) Add user interview for new module
info gathering
#26
Conversation
new module
info gatheringnew module
info gathering
Disregard the branch name - this is actually SDK-200 :) |
lib/pdk/generators/module.rb
Outdated
) | ||
rescue ArgumentError => e | ||
raise ArgumentError, e, $!.backtrace | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the purpose of this catch and re-raise?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is pretty much directly ported from https://github.com/puppetlabs/puppet/blob/master/lib/puppet/face/module/generate.rb#L120, which uses https://github.com/puppetlabs/puppet/blob/master/lib/puppet/module_tool/metadata.rb. Originally I think something in Metadata returned an Argument error rather than raising it, but I guess I changed that. Will look into this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated and removed
lib/pdk/generators/module.rb
Outdated
def self.invoke(opts={}) | ||
def self.invoke(name, opts={}) | ||
begin | ||
metadata = PDK::Module::Metadata.new.update( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe the metadata constructor should take the hash as a param that it can then merge with defaults.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@scotje what about calling update
in the constructor? There will be 4 or 5 data validation + parsing methods that need to be run for all data and I want to avoid duplicating that across the constructor and update
.
(Example: name
needs to be parsed to figure out what the namespace and author fields are from 'foo-bar')
lib/pdk/generators/module.rb
Outdated
puts "\nWhere is this module's source code repository?" | ||
metadata.data.update('source' => PDK::CLI::Input.get(metadata.data['source'])) | ||
|
||
puts "\nWhere can others go to learn more about this module?#{ metadata.data['project_page'] && " [#{metadata.data['project_page']}]" }" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it would be fine for the default to just be empty or show (none)
or something rather than having the conditional inside the string interpolation.
E.g. [#{metadata.data['project_page'] || '(none)'}]
etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated
This commit adds an interactive interview to gather module information when generating a new module. Included is a new `Metadata` class which models the eventual metadata.json file.
This commit adds an interactive interview to gather module information
when generating a new module. Included is a new
Metadata
class whichmodels the eventual metadata.json file.