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-550) Removes unrequired questions from module interview #410

Merged
merged 4 commits into from
Jan 24, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions lib/pdk/cli.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ def self.skip_interview_option(dsl)
dsl.option nil, 'skip-interview', _('When specified, skips interactive querying of metadata.')
end

def self.full_interview_option(dsl)
dsl.option nil, 'full-interview', _('When specified, interactive querying of metadata will include all optional questions.')
end

@base_cmd = Cri::Command.define do
name 'pdk'
usage _('pdk command [options]')
Expand Down
11 changes: 11 additions & 0 deletions lib/pdk/cli/convert.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ module PDK::CLI

PDK::CLI.template_url_option(self)
PDK::CLI.skip_interview_option(self)
PDK::CLI.full_interview_option(self)
flag nil, :noop, _('Do not convert the module, just output what would be done.')
flag nil, :force, _('Convert the module automatically, with no prompts.')

Expand All @@ -24,6 +25,16 @@ module PDK::CLI
raise PDK::CLI::ExitWithError, _('You can not specify --noop and --force when converting a module')
end

if opts[:'skip-interview'] && opts[:'full-interview']
PDK.logger.info _('Ignoring --full-interview and continuing with --skip-interview.')
opts[:'full-interview'] = false
end

if opts[:force] && opts[:'full-interview']
PDK.logger.info _('Ignoring --full-interview and continuing with --force.')
opts[:'full-interview'] = false
end

PDK::Module::Convert.invoke(opts)
end
end
Expand Down
6 changes: 6 additions & 0 deletions lib/pdk/cli/new/module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ module PDK::CLI

PDK::CLI.template_url_option(self)
PDK::CLI.skip_interview_option(self)
PDK::CLI.full_interview_option(self)

option nil, 'license', _('Specifies the license this module is written under. ' \
"This should be a identifier from https://spdx.org/licenses/. Common values are 'Apache-2.0', 'MIT', or 'proprietary'."), argument: :required
Expand All @@ -16,6 +17,11 @@ module PDK::CLI
module_name = args[0]
target_dir = args[1]

if opts[:'skip-interview'] && opts[:'full-interview']
PDK.logger.info _('Ignoring --full-interview and continuing with --skip-interview.')
opts[:'full-interview'] = false
end

unless module_name.nil? || module_name.empty?
module_name_parts = module_name.split('-', 2)
if module_name_parts.size > 1
Expand Down
44 changes: 25 additions & 19 deletions lib/pdk/generate/module.rb
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ def self.username_from_login
login_clean = 'username' if login_clean.empty?

if login_clean != login
PDK.logger.warn _('Your username is not a valid Forge username. Proceeding with the username %{username}. You can fix this later in metadata.json.') % {
PDK.logger.debug _('Your username is not a valid Forge username. Proceeding with the username %{username}. You can fix this later in metadata.json.') % {
username: login_clean,
}
end
Expand Down Expand Up @@ -176,6 +176,7 @@ def self.module_interview(metadata, opts = {})
validate_pattern: %r{\A[0-9]+\.[0-9]+\.[0-9]+},
validate_message: _('Semantic Version numbers must be in the form MAJOR.MINOR.PATCH'),
default: metadata.data['version'],
forge_only: true,
},
{
name: 'author',
Expand Down Expand Up @@ -249,30 +250,34 @@ def self.module_interview(metadata, opts = {})
default: [1, 2, 7],
},
{
name: 'summary',
question: _('Summarize the purpose of this module in a single sentence.'),
help: _('This helps other Puppet users understand what the module does.'),
required: true,
default: metadata.data['summary'],
name: 'summary',
question: _('Summarize the purpose of this module in a single sentence.'),
help: _('This helps other Puppet users understand what the module does.'),
required: true,
default: metadata.data['summary'],
forge_only: true,
},
{
name: 'source',
question: _('If there is a source code repository for this module, enter the URL here.'),
help: _('Skip this if no repository exists yet. You can update this later in the metadata.json.'),
required: true,
default: metadata.data['source'],
name: 'source',
question: _('If there is a source code repository for this module, enter the URL here.'),
help: _('Skip this if no repository exists yet. You can update this later in the metadata.json.'),
required: true,
default: metadata.data['source'],
forge_only: true,
},
{
name: 'project_page',
question: _('If there is a URL where others can learn more about this module, enter it here.'),
help: _('Optional. You can update this later in the metadata.json.'),
default: metadata.data['project_page'],
name: 'project_page',
question: _('If there is a URL where others can learn more about this module, enter it here.'),
help: _('Optional. You can update this later in the metadata.json.'),
default: metadata.data['project_page'],
forge_only: true,
},
{
name: 'issues_url',
question: _('If there is a public issue tracker for this module, enter its URL here.'),
help: _('Optional. You can update this later in the metadata.json.'),
default: metadata.data['issues_url'],
name: 'issues_url',
question: _('If there is a public issue tracker for this module, enter its URL here.'),
help: _('Optional. You can update this later in the metadata.json.'),
default: metadata.data['issues_url'],
forge_only: true,
},
]

Expand All @@ -282,6 +287,7 @@ def self.module_interview(metadata, opts = {})

questions.reject! { |q| q[:name] == 'module_name' } if opts.key?(:module_name)
questions.reject! { |q| q[:name] == 'license' } if opts.key?(:license)
questions.reject! { |q| q[:forge_only] } unless opts.key?(:'full-interview')

interview.add_questions(questions)

Expand Down
34 changes: 34 additions & 0 deletions spec/unit/pdk/cli/convert_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,5 +65,39 @@
}
end
end

context 'and the --skip-interview flag has been passed' do
it 'passes the skip-interview option through to the converter' do
expect(PDK::Module::Convert).to receive(:invoke).with(:'skip-interview' => true, :'template-url' => anything)

PDK::CLI.run(['convert', '--skip-interview'])
end
end

context 'and the --full-interview flag has been passed' do
it 'passes the full-interview option through to the converter' do
expect(PDK::Module::Convert).to receive(:invoke).with(:'full-interview' => true, :'template-url' => anything)

PDK::CLI.run(['convert', '--full-interview'])
end
end

context 'and the --skip-interview and --full-interview flags have been passed' do
it 'ignores full-interview and continues with a log message' do
expect(logger).to receive(:info).with(a_string_matching(%r{Ignoring --full-interview and continuing with --skip-interview.}i))
expect(PDK::Module::Convert).to receive(:invoke).with(:'skip-interview' => true, :'full-interview' => false, :'template-url' => anything)

PDK::CLI.run(['convert', '--skip-interview', '--full-interview'])
end
end

context 'and the --force and --full-interview flags have been passed' do
it 'ignores full-interview and continues with a log message' do
expect(logger).to receive(:info).with(a_string_matching(%r{Ignoring --full-interview and continuing with --force.}i))
expect(PDK::Module::Convert).to receive(:invoke).with(:force => true, :'full-interview' => false, :'template-url' => anything)

PDK::CLI.run(['convert', '--force', '--full-interview'])
end
end
end
end
17 changes: 17 additions & 0 deletions spec/unit/pdk/cli/new/module_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,23 @@
PDK::CLI.run(['new', 'module', '--skip-interview', module_name])
end
end

context 'and the full-interview flag' do
it 'passes true as the value of the full-interview option to PDK::Generate::Module.invoke' do
expect(PDK::Generate::Module).to receive(:invoke).with(hash_including(:'full-interview' => true))
expect(logger).to receive(:info).with("Creating new module: #{module_name}")
PDK::CLI.run(['new', 'module', '--full-interview', module_name])
end
end

context 'and the --skip-interview and --full-interview flags have been passed' do
it 'ignores full-interview and continues with a log message' do
expect(logger).to receive(:info).with(a_string_matching(%r{Ignoring --full-interview and continuing with --skip-interview.}i))
expect(PDK::Generate::Module).to receive(:invoke).with(hash_including(:'skip-interview' => true, :'full-interview' => false))

PDK::CLI.run(['new', 'module', '--skip-interview', '--full-interview'])
end
end
end

context 'when passed a module name with a forge user' do
Expand Down
Loading