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

Task parameters #31

Closed
smyrman opened this issue Jun 26, 2017 · 8 comments
Closed

Task parameters #31

smyrman opened this issue Jun 26, 2017 · 8 comments
Labels
type: enhancement A change to an existing feature or functionality.

Comments

@smyrman
Copy link
Contributor

smyrman commented Jun 26, 2017

As of the discussion in #29, I believe we could solve part of the problem by allowing for task parameters to be implemented with the following semantics:

  • When calling a task, either in cmds or in deps, you have the option to set/override Variables
  • Task caching mechanisms (status, sources, generates) must have access to using these variables.

Base implementation

Example syntax:

markdown:
  vars:
    FILENAME:
  cmds:
     - markdown {{.FILENAME}}.md -o {{FILENAME}}.html
  sources:
    - {{.FILENAME}}.md
   generates:
  - {{.FILENAME}}.html

single-usage:
  cmds:
    - ^markdown FILENAME=index

two-deps:
  deps:      # Allows for parallel execution
    - markdown FILENAME=index
    - markdown FILENAME=docs

Glob support & vars evaluation order

Ideally, there would also be a way to set multiple deps and/or call the task multiple times sequentially using using a glob pattern. An example glob task might look like:

markdown:
  vars:
    IN: 
    OUT: {{.IN | trimSufix ".md"}}.html   # this might be tricky to evaluate in the right order...
  cmds:
     - markdown {{.IN}} -o {{.OUT}}
  sources:
    - {{.IN}}
   generates:
  - {{.OUT}}

cmd-usage:
  cmds:
    - ^markdown IN=src/*

two-deps:
  deps:
    - markdown IN=src/*

For the "vars" section in this example, it might be necessary that vars are handled in the order they are listed, which, depending on implementation here, might be hard if Go maps are used. It could also be that YAML don't like that the IN variable is empty.

If this proves to be a problem, and we also want to enforce that all task parameters are set, we could consider to add an additional keyword "args", where the semantics are to allow args to be set before vars:

markdown:
  args:
    - IN
  vars:
    OUT: {{.IN | trimSufix ".md"}}.html
  cmds:
     - markdown {{.IN}} -o {{.OUT}}
  sources:
    - {{.IN}}
  generates:
    - {{.OUT}}
@andreynering andreynering added the type: enhancement A change to an existing feature or functionality. label Jun 26, 2017
@smyrman
Copy link
Contributor Author

smyrman commented Jun 27, 2017

Potential solution to var evaluation order based on using a sub-task for it:

default:
  deps:
    - ^markdown IN=src/*.md

markdown:
  deps:
    - setout # should get IN passed along.
  cmds:
     - markdown {{.IN}} -o {{.OUT}}
  sources:
    - {{.IN}}
  generates:
    - {{.OUT}

setout:
  vars:
    OUT: {{.IN | trimSufix ".md"}}.html

This saves us from adding a new keyword args, but as a tradeoff, there is no way to enforce that all arguments are set. As another tradeoff, we need to add an extra task to set environment variables. I think it's probably an acceptable tadeof. @andreynering, what do you think?

Not that we necessarily need glob support as input from the start, but it's nice to know that the design we choose from the start will be capable of supporting it.

@andreynering
Copy link
Member

@smyrman Advanced cases seems to be a bit tricky. I think I'll start this in another branch and ask you (and other) for feedback before it is stable and can me merged on master.

I think in this case you should probably repeat the glob:

task:
  cmds: [...]
  vars:
    IN: {{glob "./**/*.md"}}
    OUT: {{glob "./**/*.md" | trimSuffix ".md"}}

Note the glob func doesn't exist yet, but we can program that.

@smyrman
Copy link
Contributor Author

smyrman commented Jun 27, 2017

I think in this case you should probably repeat the glob

I guess you want/have to set this when you call the task though?

two-deps:
  deps: 
    - markdown IN={{glob "./**/.md"}} OUT={{glob "./**/*.md" | trimSuffix ".md"}}.html

Even in this case, if glob returns a slice, I don't know what trimSuffix would do, and I don't think the .html would be appended as if it was a string.

Not saying this example syntax is necessarily what we should go for, but part of the goal would be to allow for that one to one mapping within the task itself.

Think I'll start this in another branch and ask you (and other) for feedback before it is stable and can me merged on master.

Awesome 👍

@smyrman
Copy link
Contributor Author

smyrman commented Jun 29, 2017

Just to be clear, all we specifically need atm. would be the base implementation, without any GLOB support or similar, so that's a very good start.

The advanced usage can come later.

@andreynering
Copy link
Member

@smyrman Agreed. I don't think the basic idea is hard to implement. I may give a try this weekend.

@andreynering andreynering mentioned this issue Jul 1, 2017
4 tasks
@andreynering
Copy link
Member

@smyrman Basic implementation available in #32 and parameters branch.

@smyrman
Copy link
Contributor Author

smyrman commented Jul 2, 2017

I will have a look:-)

@andreynering
Copy link
Member

Done on master

If there is any improvement still not possible, please open another issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: enhancement A change to an existing feature or functionality.
Projects
None yet
Development

No branches or pull requests

2 participants