-
Notifications
You must be signed in to change notification settings - Fork 0
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
Add command to configure project for order files #7
Conversation
desc 'Configure order files for iOS' | ||
|
||
# Optional options | ||
option :only_enable_linkmaps, type: :boolean, required: false, desc: 'Only enable linkmaps' |
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 option is a bit confusing, not sure what it's doing.
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 see below it's related to the download script, maybe that could instead be skip_download_script
?
Logger.info " Creating script 'Download Order Files'" | ||
phase = target.new_shell_script_build_phase('Download Order Files') | ||
phase.shell_script = "\ | ||
if [ \"$CONFIGURATION\" != \"Release\" ]; then |
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.
Would using a heredoc avoid all the escaping, or is that necessary for the script itself?
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.
Never tried heredoc, time to test it
# Create the script phase if it doesn't exist | ||
phase = target.shell_script_build_phases.find { |item| item.name == 'Download Order Files' } | ||
if phase.nil? | ||
Logger.info " Creating script 'Download Order Files'" |
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.
nit: I would prefix with our name, e.g. EmergeTools Download Order Files
lib/emerge_cli.rb
Outdated
@@ -30,6 +31,7 @@ module EmergeCLI | |||
|
|||
register 'configure' do |prefix| | |||
prefix.register 'snapshots-ios', Commands::Config::SnapshotsIOS, aliases: ['c'] | |||
prefix.register 'order-files-ios', Commands::Config::OrderFilesIOS, aliases: ['o'] |
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.
nit: I'm thinking maybe we want to avoid aliases for the subcommands and instead have one for configure
? Also the c
above for 'snapshots-ios' is probably a mistake.
No description provided.