maid icon indicating copy to clipboard operation
maid copied to clipboard

rename `beforeAll/afterAll` to `pre/post`?

Open ianstormtaylor opened this issue 6 years ago • 5 comments

Hey, thanks for the interesting tool!

I was just gonna make a suggestion. Right now you can have things like postbuild and prebuild which run automatically. But the automatic before/after everything is confusingly named afterAll and beforeAll, which isn't consistent.

It might be nice to allow pre and post tasks that run before/after everything instead, to stay consistent. It feels natural that pre without pre{task} would count for everything.

ianstormtaylor avatar May 31 '18 15:05 ianstormtaylor

Interesting suggestion.

While that does a good job of aligning with the naming convention, I feel like it could be confusing about the context of when it runs. Is it pre each task? As in, does in run before anything else runs. Or is it pre all tasks? I know in this context it applies to the latter, but a task named pre isn't very descriptive to the on-looker.

I wonder if instead we could make a plain english semantic to marry with the Run task command.

i.e.

## Clean

Run after task `build`
Run before all tasks

zephraph avatar Jun 03 '18 21:06 zephraph

@zephraph I'm not sure I totally understand. I think "before anything else runs" and "pre all tasks" are kind of synonymous. I'd personally avoid any of the plain english stuff, since I think it makes the API really hard to remember.

ianstormtaylor avatar Jun 04 '18 13:06 ianstormtaylor

I disagree @ianstormtaylor. It should be plain English. That's what makes this so interesting.

However it should still be a standard API.

<action> <condition> <target> [target identifier] <runtime params...>

In our case, these are:

run [before|after] [task|all]

Or run before all, run after all

chrisdmacrae avatar Jun 05 '18 03:06 chrisdmacrae

Upon thinking about the above more, it wont work.

How do I run multiple "before all tasks" before themselves?

Having a single command called "beforeAll", "afterAll" makes sense, because there should only be one of each.

chrisdmacrae avatar Jun 05 '18 23:06 chrisdmacrae

You could still do a label, just error out if there's more than one. I think the current beforeAll, afterAll semantics are fine too though.

zephraph avatar Jun 06 '18 00:06 zephraph