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

rename beforeAll/afterAll to pre/post? #7

Open
ianstormtaylor opened this issue May 31, 2018 · 5 comments
Open

rename beforeAll/afterAll to pre/post? #7

ianstormtaylor opened this issue May 31, 2018 · 5 comments

Comments

@ianstormtaylor
Copy link

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.

@zephraph
Copy link
Contributor

zephraph commented Jun 3, 2018

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

@ianstormtaylor
Copy link
Author

@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.

@chrisdmacrae
Copy link

chrisdmacrae commented Jun 5, 2018

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
Copy link

chrisdmacrae commented Jun 5, 2018

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.

@zephraph
Copy link
Contributor

zephraph commented Jun 6, 2018

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.

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

4 participants