Add brunch support #239
Add brunch support #239
Conversation
There was a problem hiding this comment.
Hello @ptondereau,
Thanks for this PR, I did not know about brunchyet.
There are some small remarks that need to be fixed before we can merge this one in.
Can you take a look at them?
|
|
||
| *Default: null* | ||
|
|
||
| If your `brunch-config.js file is located at an exotic location, you can specify your custom gulp file location with this option. |
There was a problem hiding this comment.
This line contains "custom gulp file" which is a copy/paste from the gulp task?
| tasks: | ||
| brunch: | ||
| brunch_file: ~ | ||
| task: ~ |
There was a problem hiding this comment.
The default configured task is 'build'.
|
|
||
| **task** | ||
|
|
||
| *Default: null* |
There was a problem hiding this comment.
The default configured task is 'build'.
| } | ||
|
|
||
| $arguments = $this->processBuilder->createArgumentsForCommand('brunch'); | ||
| $arguments->addOptionalArgument('%s', $config['task']); |
There was a problem hiding this comment.
Maybe the task option should be required? Otherwise if the task is empty, the path will be added at the position of the task name which will result in an error.
|
@veewee thank you for your feedback, I'll fix all of this plus add some more advanced parameters this weekend. |
|
@veewee good to me |
| task: build | ||
| env: production | ||
| jobs: 4 | ||
| debug: false | ||
| triggered_by: [js, jsx, coffee, ts, less, sass, scss] | ||
| ``` | ||
|
|
||
| **brunch_file** |
There was a problem hiding this comment.
The brunch_file section is removed from the task.
Is there a reason why it is removed or can it be re-added?
If no, then this section in the documentation should be removed.
There was a problem hiding this comment.
brunch_file doesn't exist anymore since 2.0, i'll remove it.
| $arguments->addOptionalArgument('%s', $config['task']); | ||
| $arguments->addOptionalArgument('%s', $config['brunch_file']); | ||
| $arguments->addRequiredArgument('%s', $config['task']); | ||
| $arguments->addOptionalArgument('--env %s', $config['env']); |
There was a problem hiding this comment.
This will result in the escaped CLI argument '--env production', shouldn't this be 2 separate arguments like this: '--env' 'production'?
If so, you could use the $arguments->addOptionalArgumentWithSeparatedValue() method.
There was a problem hiding this comment.
It should result to $ brunch build --env production --> --env 'production'
There was a problem hiding this comment.
Then I guess you should use addOptionalArgumentWithSeparatedValue() instead.
|
@veewee another check please |
|
It looks good, thanks! |
My PR add support of js building tools Brunch which become more and more popular (such as Gulp and Grunt)
New Task Checklist:
run()method readable?run()method using the configuration correctly?