npm-watch icon indicating copy to clipboard operation
npm-watch copied to clipboard

Added support for task arguments

Open stevenvachon opened this issue 7 years ago • 6 comments

This is waiting on #46


This change is Reviewable

stevenvachon avatar Jun 23 '17 19:06 stevenvachon

Overall this looks good, but I found the following edge cases that would still need to work:

  • npm run watch should simply run all tasks defined
  • npm run watch -- ...args should run all tasks defined passing in the args to each one

here is the setup I was using to test it package.json:

{
    "name": "npm-watch test",
    "version": "1",
    "devDependencies": {
        "npm-watch": "file:path to local version of npm-watch"
    },
    "scripts": {
        "watch": "npm-watch",
        "echo": "node index.js",
        "echo2": "node foo.js"
    },
    "watch": {
        "echo": "index.js",
        "echo2": "foo.js"
    }
}

index.js

function foo(arg) {
    console.log(arguments);
}
foo(process.argv.slice(2));

foo.js

function foo(arg) {
    console.log(arguments);
}
foo(process.argv.slice(2).reverse());

M-Zuber avatar Jun 26 '17 13:06 M-Zuber

We should add a test suite.

stevenvachon avatar Jun 27 '17 13:06 stevenvachon

We should add a test suite.

That is something I would love to do, if you have an idea on how to implement, please open an issue/pr so we can discuss it in a dedicated location

M-Zuber avatar Jun 27 '17 14:06 M-Zuber

I'd like to do it here in a separate commit, as it'll help me be sure that my PR passes your above case.

stevenvachon avatar Jun 27 '17 15:06 stevenvachon

Generally I would prefer to separate something like that out to a dedicated PR, but as long as each commit is self contained, go ahead and do it on this PR

M-Zuber avatar Jun 27 '17 15:06 M-Zuber

Travis and AV are both connected now

M-Zuber avatar Jun 28 '17 07:06 M-Zuber