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

Alternative way to construct a task object #49

Closed
satazor opened this issue Jan 27, 2013 · 32 comments
Closed

Alternative way to construct a task object #49

satazor opened this issue Jan 27, 2013 · 32 comments
Assignees
Milestone

Comments

@satazor
Copy link
Member

satazor commented Jan 27, 2013

This is a suggestion made by @mklabs and its actually pretty interesting. I like it.
His suggestion is basically to provide a programatic API to construct the task object that automaton already knows. Here's an example:

var task = module.exports = require('task-builder').create();
var otherTask = require('other-task');

 task
  .id('my-task')
  .name('My awesome task')
  .option('a', 'Option a', 'foo')
  .subtask('cp', { files: { 'foo/' : 'bar/' })
  .subtask(function (opts, ctx, next) { next(); })
  .subtask(otherTask, { foo: 'bar' })
  // ..etc;

Needs further discussion. This alternative syntax is much less verbose.

@sindresorhus
Copy link

I ❤️ it! I've had similar thoughts for grunt.

@marcooliveira
Copy link
Member

👍

This suggestion looks really nice. I like the simplicity. I also like that it is fully backwards compatible, as we're not changing the syntax, just using the language to build the same objects.

Come to think of it, the only reason why we declare tasks the way we do, is due to the fact that initial tasks were simple JSON files, which eventually evolved into JS files.

We need to discuss this properly. If this approach proves to be more effective and comfortable, we could even make it the standard.

@satazor
Copy link
Member Author

satazor commented Jan 28, 2013

If we decide to implement this, we must define the API properly to satisfy/generate the current object syntax and to provide extension points for future changes.

@millermedeiros
Copy link

+1 plain objects are hard to read when it's too long and you have too many nested elements.. can't figure out where a tasks ends and the next one begins..

it will also make it easier to spot typos (since it will throw errors) and work better with autocomplete depending on the editor. It's also what the cool kids been doing these days 😜

@satazor
Copy link
Member Author

satazor commented Feb 2, 2013

So it seems we have a go on this. I will propose the builder API soon so you guys can evaluate.

@satazor
Copy link
Member Author

satazor commented Feb 3, 2013

Here's what I come up with:

{
    id: function (id) {},
    name: function (name) {},
    description: function (desc) {},
    author: function (author) {},

    option: function (name) {},
    option: function (name, desc) {},
    option: function (name, desc, defaults) {},

    filter: function (func) {},
    job: function (id|task|func) {},
    job: function (id|task|func, config) {},
};

Sample task (note that cp-dir is an imaginary loaded task):

var otherTask = require('otherTask');

module.exports = function (task) {
    task
    .id('test')
    .name('Test')
    .description('Do things')
    .author('André Cruz')
    .option('foo', 'The foo option', 'bar')

    .filter(function (opts, ctx, next) {
        // Do things before the actual task jobs runs
        next();
    })

    .job('cp-dir', {
        options: {
            dir: '{{foo}}'
        }
    })
    .job('cp-dir', {
        description: 'Copying some other useful dir',
        options: {
            dir: '{{foo}}'
        }
    })
    .job(otherTask, {
        description: 'Do something useful',
        fatal: false,
        mute: true,
        options: {
            option1: '{{foo}}'
        }
    })
    .job(function (opts, ctx, next) {
        // Do things
        next();
    });

    return task;  // This would be optional
};

Two things that bother me:

  • Extensibility of option(). For instance, if we plan to add ability to specify option types (Add ability to specify options types to automatically assert them #31), then the signature will have 4 arguments. Seems too long.
  • Boilerplate necessary with the module.exports = function (task). I made it like this because we don't want devs to require the builder. If they had to require it, they also add to include the builder in their package.json and would slow down the installation of tasks overall.

@satazor
Copy link
Member Author

satazor commented Feb 3, 2013

An alternative signature to job():

job: function (id|task|func) {},
job: function (id|task|func, opts) {},
job: function (id|task|func, description, opts) {},
job: function (id|task|func, description, opts, flags) {},

Examples:

.job('cp-dir')
.job('cp-dir', {
    dir: '{{foo}}'
})
.job('cp-dir', 'Copying dir', {
    dir: '{{foo}}'
}),
.job('cp-dir', 'Copying dir', {
    dir: '{{foo}}'
}, { mute: true, fatal: false ))

This alternative has a much more complex/confusing signature but tends to be less verbose and easier to read for 90% of use cases (being the second and third, the common ones).

@guybedford
Copy link

I like this alternative signature suggestion - .job('cp-dir', { dir: '{{foo}}' }) seems to be the least verbose, as one doesn't need to rewrite "option" each time.

@sindresorhus
Copy link

I prefer the first one. Even though it's a tiny bit more verbose it's a lot clearer and consistent.

@marcooliveira
Copy link
Member

Even though the second alternative is a bit less verbose, the first seems easier to read, and less of a "oh, we forgot about something, lets just add another argument", and presents a more unified way of passing behaviour modifiers (options, mute, fatal, description, anything else we come up with).

Summing it up, I would go with the first.

@satazor
Copy link
Member Author

satazor commented Feb 4, 2013

@conradz what's your opinion?

@marcooliveira
Copy link
Member

Btw, I would like to present an alternative name to job, which would be do. Less characters, and doesn't convey a sense that it's a separate job, but a TODO list.

@conradz
Copy link

conradz commented Feb 5, 2013

I would be +1 on the first signature of job, since it is more of a unified way of passing options, although the second is very readable IMO, so no big deal. Definitely +1 on do instead of job, since that is one of the most confusing things that I ran into getting up to speed on it (declaring jobs vs running jobs).

@satazor
Copy link
Member Author

satazor commented Feb 5, 2013

I also think the first one will be better in the long run. Also like do instead of job. Though, jshint complains it is a reserved word. Thing is that it should be valid to do obj.do(), at least browsers do not complain.. but...

@satazor
Copy link
Member Author

satazor commented Feb 5, 2013

What you guys think about runs() or does()?

@sindresorhus
Copy link

According to JSHint author it's a known bug that's soon to be fixed. Let's go with obj.do()

@satazor
Copy link
Member Author

satazor commented Feb 6, 2013

It seems that the ability of using reserved words as property names is a ecma5 feature:

So, will go forward with do.

@marcooliveira
Copy link
Member

👍 also, regarding the separate package for the task builder, as we discussed, I agree that it should be built in with automaton, since task syntax is strictly coupled with the automaton version.

@conradz
Copy link

conradz commented Feb 7, 2013

👍 this will make it much nicer to use

@filipediasf
Copy link
Member

👍 sounds good to me :)

@ghost ghost assigned marcelombc Feb 8, 2013
@tkellen
Copy link

tkellen commented Feb 8, 2013

Is there any chance you all would be willing to work with @cowboy and I on http://github.com/tkellen/node-task? I would really like to see a standard task API that is a) not dependent on a task runner and b) works across the entire node ecosystem.

@satazor
Copy link
Member Author

satazor commented Feb 8, 2013

@tkellen I've already seen the spec. We got some similarities in there. Things that would need to be solved:

  • The spec does not define options that a task has. This is useful for both printing options in the CLI and for devs to look at.
  • Each automaton task is built upon subtasks (jobs). A subtask can be a function, a loaded task (via string) or a task required via node's require. This allows tasks to reuse others seamlessly.
  • Many people like this programmatic API instead. Is more nodeish.

@tkellen
Copy link

tkellen commented Feb 8, 2013

  • I'd be open to suggestions on defining options. This spec is very young.
  • To my mind, these requirements reside at the task-runner level, whose job is to compose and execute node-task compliant tasks. I don't see any immediate conflict?
  • node-task defines a spec, you can use whatever you like to build a task as long as it produces a compatible api.

@marcooliveira
Copy link
Member

@tkellen we might be open to that, but there are some things that trouble me. Just like @satazor mentioned, there are some core concepts in which automaton differs from other task runners. If we all come down to a common place, that place might limit some. We're just as worried with standardising a good, extensible API for tasks as you, reason why we have some open discussions :)

I have to leave now, but I'll take a look at the spec this weekend. Meanwhile, if you're interested, take a look at our current plans for the automaton API. We're making efforts to make it less verbose, without loosing flexibility, with nice extension points.

@marcooliveira
Copy link
Member

Oh, forgot to ask. Is the objective of the spec to achieve interoperability between task runners?

@tkellen
Copy link

tkellen commented Feb 9, 2013

@marcooliveira

Yes, you could say the objective is to achieve interoperability between task runners. I would take it even further and say that the objective is to take build processes and make them consumable modules that don't even require a task runner (though in basically all cases you would want to use one, as manually configuring a node-task compliant task to run would be super annoying).

At the moment, the primary driver of the spec is Grunt (I'm the co-maintainer), but I am 100% open to including more players. @paulmillr of brunch is already on board to work with us. The grunt ecosystem is pretty huge--I'd like to see all of that developer effort consumable on the largest scale possible.

Do you have an example of something you're trying to do that doesn't fit within node-task?

@conradz
Copy link

conradz commented Feb 9, 2013

I think an interoperable definition of tasks would fit really well into automaton, even with some of automaton's differences with other task runners. The tasks that the interoperable definition is covering are generic tasks that can be used anywhere. Automaton would then allow a user to define how and in what order to run these tasks. The tasks that would be interoperable would be tasks similar to the standard tasks included with automaton currently, say a less task or a copy-dir tasks. Automaton then would basically allow the user to provide input and configuration to the tasks, and then run them. Automaton would still be based on sub-tasks, but these sub-tasks could be interoperable tasks, or other automaton tasks that define other sub-tasks. I really don't see any reason that would make it terribly hard to integrate interoperable tasks.

@satazor
Copy link
Member Author

satazor commented Feb 16, 2013

The builder integration is done, will make some additional tests. Here's the final API:

{
    id: function (id) {},
    name: function (name) {},
    description: function (desc) {},
    author: function (author) {},

    option: function (name) {},
    option: function (name, desc) {},
    option: function (name, desc, default_value) {},

    setup: function (func) {},
    teardown: function (func) {},

    do: function (id|task|func) {},
    do: function (id|task|func, config) {}
};

The filter was removed in favor of setup and teardown. We feel these are better names and are consistent with the upcoming node-task spec.

@tkellen
Copy link

tkellen commented Feb 16, 2013

Just FYI, we are officially launching Grunt 0.4 on Monday, after which point the real work on node-task will begin. I'd love to have your team involved!

@satazor
Copy link
Member Author

satazor commented Feb 17, 2013

Done!

@satazor satazor closed this as completed Feb 17, 2013
@marcooliveira
Copy link
Member

@tkellen Thanks for the update. We've given some thought regarding the spec, and we definitely see benefits in adopting and participating in the spec. We're brainstorming right now, either me or @satazor should meanwhile open a discussion with our thoughts in the repository.

Just a quick note, we feel that the documentation seems a bit complicated, and there are some mixed concepts in there. Might be worth tackling this, so others can dive in easier. We're happy to get on board. Let's move this discussion there, since this ticket is now closed, and does not fully relate to the actual spec.

@tkellen
Copy link

tkellen commented Feb 21, 2013

I agree, my implementation of the spec shouldn't be co-mingled with the spec itself, but I was originally using it as a tool to flesh out the ideas I had in mind. I will be separating them soon.

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

No branches or pull requests

9 participants