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

Allow parameters in command definitions in runner.yml #111

Closed
donquixote opened this issue Jan 23, 2020 · 4 comments · Fixed by #120
Closed

Allow parameters in command definitions in runner.yml #111

donquixote opened this issue Jan 23, 2020 · 4 comments · Fixed by #120

Comments

@donquixote
Copy link

In #25 the option was added to define commands inside the runner.yml(.dist) file.

It would be nice if we could also add parameters to those commands.

Currently this has to be defined like so, by specifying the full cli command as a single string:

commands:
  efd:build-themes:
  - "${runner.bin_dir}/run efd:npm-build --npm-dir=path/to/npm/root"

With support for parameters / arguments, it could look like this:

commands:
  efd:build-themes:
  -
    task: "run"
    command: "efd:npm-build"
    arguments: {npm-dir: "path/to/npm/root"}

(The example is inspired from this, https://github.com/ec-europa/ecdc-vaccine-reference/blob/6cf99aec072e0b6616cc6a0967be692692c53103/src/TaskRunner/Commands/EcdcBuildCommands.php#L71)

@donquixote
Copy link
Author

The relevant piece of code is in CollectionFactory::taskFactory()

(I am posting specific commit ids to be sure this link still points to the same piece of code in the future)

@donquixote
Copy link
Author

One could argue that the "old" version is actually shorter.
But the second version is more semantic and readable.

@claudiu-cristea
Copy link
Contributor

Those are more "options" than "arguments".

@claudiu-cristea
Copy link
Contributor

@donquixote the link you've provided in the description, is broken. Could you please send here the correct link or the code snippet. I would like to fill a PR.

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 a pull request may close this issue.

2 participants