-
Notifications
You must be signed in to change notification settings - Fork 177
Move <target> option to <commands> #31
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
Comments
Looks good too, we need to mockup a bigger file, to check how that new style could help us |
This would create much more complexity to the code (reading commands vs. targets from YAML) and I don't think it helps Supfile readability much. We made the distinction between commands and targets for reason. Target is just an alias for multiple commands that are run sequentially as opposed to commands. One should not be able to put another Think of: commands:
prepare:
desc: Prepare to upload
run: npm run build
network: local
upload:
desc: Upload this repository
upload:
- src: ./
dst: /tmp/$IMAGE
deploy:
desc: Deploy
run: echo "Should this be run? And when?"
commands: # effectivelly "sub-commands"
- prepare
- upload |
I kind of like it.. I was thinking the same thing recently. It would take a little bit of work, but not that much since "commands" is its own field, YAML will parse it fine. We could define that the "run" will always execute before all other commands. |
I'm not convinced this is worth the effort. And there are ugly edge-cases like: commands:
loop:
run: "Running loop..."
commands:
- loop Not to mention, this breaks the Supfile API. |
I don't think its a rush, but we can leave the ticket open for future ideas. To prevent the loop, we could check that when we build the command tree easily. It does offer us to have command groups easily. Ie. declare a bunch of single commands, make smaller groups, then "deploy" will call those other groups.. the commands execution tree. |
Ok, cool. I will think of this as part of big refactor for v1.0. |
Sure it can generate some ugly edge-case, as well will take the code to another level of complexity, like adding lots of security checks. IMHO now the project needs a stable version before adding more features, the community have found some bugs and maybe have a little more, depending of the case of course. Agree that project needs a refactor before new specs in |
@eduardonunesp sure, I'll create a PR for the big refactor once I have more time and it will be open for discussion before we merge anything :) Any help will be appreciated. No big rush for now, though. I'm glad we're thinking about new features thoroughly. |
deploy:
desc: Deploy
run: echo "Should this be run? And when?"
commands: # effectivelly "sub-commands"
- prepare
- upload No, deploy:
targets:
- prepare
- upload Consequently we have in config only 3 entities on first level:
👍 |
Some improvements :)
Was:
Became:
What do you think about it?
The text was updated successfully, but these errors were encountered: