Fixes bug with release being undefined and adds --no-debug to production run #307
Conversation
This allows it to work with LLVM 4.0 on linux.
elorest
changed the title
src/amber/cli/commands/run.cr
Outdated
| build_options = Array(String).new | ||
| build_options << "--release" if options.e.downcase == "production" | ||
| build_options << "--no-debug" if options.e.downcase == "production" | ||
| build_options << " " if build_options.size > 0 |
There was a problem hiding this comment.
This may be done in a bit cleaner manner:
if options.e.downcase == "production"
build_options.concat ["--release", "--no-debug", " "]
endThere was a problem hiding this comment.
I wrote it this way so that we could add different options for different reasons but only have a space if there was at least one option.
If it will never change we could just write build_options += "--release --no-debug "
There was a problem hiding this comment.
Right, but .downcase and == "production" are about to execute twice, what is not really good
There was a problem hiding this comment.
I agree with the above is much cleaner. I will also remove the additional space " " and just add the space on line 28 between the } $
There was a problem hiding this comment.
But then it ends up with 2 spaces in the case that the env isn't production.
src/amber/cli/commands/run.cr
Outdated
| build_options = Array(String).new | ||
| build_options << "--release" if options.e.downcase == "production" | ||
| build_options << "--no-debug" if options.e.downcase == "production" | ||
| build_options << " " if build_options.size > 0 |
There was a problem hiding this comment.
I agree with the above is much cleaner. I will also remove the additional space " " and just add the space on line 28 between the } $
…ion run (amberframework#307) * Adds --no-debug to production run. This allows it to work with LLVM 4.0 on linux. * initializing array(string) * added support for issue amberframework#308 and cleaned up
Description of the Change
This allows it to work with LLVM 4.0 on linux.
Also saves a bit of time on compiling.
Fixes bug where if run command was used for development or test it would error because #{release} didn't exist.
Fixes issue #308