Add CLI config overrides #126
Conversation
* master: Remove Travis config for now Add basic travis config Update liftoffrc(5) for script phase changes Add configuration for setting up Run Script phases Reluctantly allow the use of tabs for indentation Be more specific about not linking Info.plist file Centralize deployment target
* master: Break configuration attributes into multiple lines Add deprecation manager
|
Thoughts on this? My main concern with this approach is all of the duplication. Every But all the other ways I can think of to DRY this up either have less flexibility or unnecessarily complicate the code. |
|
Yeah, sorry. Not ignoring, just got busy with some other stuff. That's my main concern as well. But like you said, I don't think there's any good way around that. Let me take another look through this tomorrow. (Or maybe tonight, if I get time) |
|
no worries, I just wanted to get a discussion started |
| @@ -2,11 +2,12 @@ module Liftoff | |||
| class CLI | |||
| def initialize(argv) | |||
| @argv = argv | |||
| @liftoffrc = {} | |||
There was a problem hiding this comment.
Lets call this something else since it's not actually coming from the liftoffrc. Maybe flags or options?
|
We should remove the option to enable/disable the todo scripts since those have been replaced by I'm wondering if there shouldn't be a different line drawn in the sand for what keys to add here. Basing the availability of the command line flags on the type of the value from the The stuff that we prompt for, git, and CocoaPods makes sense to enable/disable, but everything else feels like low enough overhead to ignore. Thoughts? ping @thoughtbot/ios |
|
|
||
| # Boolean Options | ||
|
|
||
| opts.on('--[no-]cp', 'Enable/Disable Cocoapods') do |use_cocoapods| |
There was a problem hiding this comment.
Can we use --[no-]cocoapods and alias it to -c?
There was a problem hiding this comment.
Ah, I see. --company is already using -c. Lets not alias --cocoapods then. I'd rather the long flags be explicit. Worst case scenario, this gives me a reason to learn how to write completion functions and then probably cry a lot I bet.
* master: Update README
- removed/renamed some CLI options - moved liftoffrc/cli_options merging from LaunchPad to ConfigurationParser
| @@ -10,7 +14,8 @@ def project_configuration | |||
| def evaluated_configuration | |||
| default_configuration. | |||
| merge(user_configuration). | |||
| merge(local_configuration) | |||
| merge(local_configuration). | |||
There was a problem hiding this comment.
Place the . on the next line, together with the method name.
There was a problem hiding this comment.
This is a new addition to our style guide, but it would probably be good to go ahead and make this change now since we're messing around in here.
| @@ -1,6 +1,10 @@ | |||
| module Liftoff | |||
| class ConfigurationParser | |||
|
|
|||
| def initialize(cli_options) | |||
There was a problem hiding this comment.
Can we call this argument options consistently? Here as well as in Launchpad?
|
I think this is good to go after these last 2 nitpicks. Thanks for being tackling this! |
- renamed instances of "cli_options" and "cli_config" to just "options"
|
Done and done. |
|
Rebased onto master and merged in 080b30d |
Adds ability to override all boolean, integer & string liftoffrc config options with CLI arguments. Fixes #124.