-
Notifications
You must be signed in to change notification settings - Fork 2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Allowing an environment variable of MINIFY=false to disable minificat…
…ion of the build:browser cake task.
- Loading branch information
Showing
1 changed file
with
4 additions
and
3 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
7ae284f
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.
Eh, this is pretty ugly. Wouldn't it be better if there was a separate cake task for minifying the browser build and this task invoked that one? Then we could just implement command-line options like in rake tasks (scroll to "Tasks with Arguments" section) to specify when we want to skip the minification.
Or, if we didn't want to implement argument-passing, we could just create a build task, a minification task, and a build_and_minify task, which just runs the other two successively. Both of these solutions seem more elegant to me than the one implemented in this commit.
7ae284f
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 disagree ... environment variables are often how arguments are passed into shell scripts -- but I'd also be happy to have an implementation of tasks-that-take-arguments, as long as we can make it nicer than rake arguments (which are pretty awful).
7ae284f
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 agree that rake arguments aren't the best, so I'd be in favor of an implementation of the second method I proposed. Are you in agreement, jashkenas?
7ae284f
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.
Not at all. Deciding whether to minify or not is very much an option of the
build:browser
task -- we really don't need to litter the task list with variants of it. I think that environment variables are fine, as are arguments, if we have a nice implementation.