Skip to content
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

Naming of WatchTask(s) #225

Merged
merged 2 commits into from Jul 4, 2014
Merged

Naming of WatchTask(s) #225

merged 2 commits into from Jul 4, 2014

Conversation

doublerebel
Copy link
Contributor

PackageTask and PublishTask will both take a name. WatchTask has a fixed name of 'watch'. For better namespacing and versatility, I support naming WatchTasks.

This is a breaking change in syntax. However, WatchTask is relatively young (see: #122), and I searched Github but found no projects that depend on the current WatchTask syntax. I think the change is reasonable since it brings the method arguments more in line with other Tasks.

@cedx
Copy link

cedx commented Jul 4, 2014

I think it's a good idea, because it allows us to define several watch tasks (e.g. one for CSS preprocessing, one for JS concatenation, and a last one combining the 2 previous).

Your pull request introduces a breaking change, but it can be avoided by using parameter swapping, like this naive implementation:

if(Array.isArray(name)) {
  definition = taskNames;
  taskNames = name;
  name = 'watch';
}

@mde
Copy link
Contributor

mde commented Jul 4, 2014

I have no idea how this got dropped. This is absolutely something people have asked for on other occasions. I'll get this fixed up and merged ASAP. :)

@mde mde merged commit 15b24ca into jakejs:master Jul 4, 2014
@mde
Copy link
Contributor

mde commented Jul 4, 2014

Merged, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants