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

[WIP] Implement task params in spec #126

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open

[WIP] Implement task params in spec #126

wants to merge 4 commits into from

Conversation

leemeichin
Copy link

@leemeichin leemeichin commented May 4, 2017

Thought: is positional substitution preferable to named variables? e.g. {{ .ParamName }} or $paramName seems easier to follow than $1, $2, $3, etc...

@xsb
Copy link
Member

xsb commented May 4, 2017

Hi @leemachin I will be out for a few days, will check this early next week.

Thanks so much for jumping in! I want to start spending more time with the project again.

@leemeichin
Copy link
Author

No worries @xsb :) there's not much to look at now, I'm just opening it early to keep a track of what happens with it.

@leemeichin
Copy link
Author

Most of the parsing side is all sorted out. All that's left is the complicated stuff :)

  • passing that in from the CLI. It looks like the format would used named parameters like dog task --arg value --arg-2 value-2 ..., where --arg corresponds to the name of the param in the Dogfile. Otherwise the CLI arg parsing code would need a heavy rewrite to allow for positional params like dog test integration. I'm not sure what the existing logic with taskArgs is doing.

  • injecting it into the command using either the position defined by the order of the params in the yaml: $1, $2...$n, or using some kind of templating to swap out the names if they exist: {{param-one}}, {{param-two}}.

Use a CLI parsing lib to build a command suite from the dogfile, as opposed to parsing args and seeing how they might map. This means a dogfile can potentially pass barewords args as well as flags. (e.g. `dog deploy master` as opposed to `dog deploy --branch=master`).
@leemeichin
Copy link
Author

@xsb I'm going to push up WIP commits showing what I'm thinking with this (with most of it living in experimental_cli.go).

Basically, the current structure manually parses ARGV and does different things depending on the first character it detects (is it a flag, is it a task?). It does this before it even tries to parse the dogfile, which means that it has to parse and store args that might turn out to be invalid.

I've pulled in a CLI library to help structure so essentially you are building an entire CLI tool from a YAML definition. This inverts that relationship so the dogfile needs to be valid before anything else, and it's used to build up the suite of commands. I felt this has made it easier to follow compared to doing it all from scratch, at the expense of pulling in an external dependency.

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.

2 participants