lukeed / taskr

A fast, concurrency-focused task automation tool.
MIT License
2.53k stars 74 forks source link

Ignore cli arguments after double-hyphen #176

Closed danny-andrews closed 7 years ago

danny-andrews commented 8 years ago

This change allows users of fly to pass command line arguments for their tasks to consume, provided they are listed after a '--'. This is similiar to how npm scripts work: http://bit.ly/1W3ddqA.

Refs #164.

Documentation for the '--' minimist option can be found here.

lukeed commented 8 years ago

Have you tested the CLI commands in addition to this? Our tests don't really cover them all. Eg:

fly -f other/dir build -- value
lukeed commented 8 years ago

@danny-andrews @brj I'm seeing less use for this. 🎱 Am I missing something obvious?

tjbenton commented 8 years ago

This would be a better option for passing in arguments than fly build:production because that way you wouldn't be able to pass in production if you just ran the fly default command.

Akkuma commented 7 years ago

So I wanted to pipe in and say we were looking to do this in our codebase, the idea is the following:

We have builds that target different environments and it would be optimal to only have 1 task to handle essentially the same builds. The environment would be used for the webpack.config to determine things. I "hacked" something like this in essentially when I manually started Fly in v1.3 and leverage the value property.

lukeed commented 7 years ago

@Akkuma is it possible to achieve the same with node environment variables?

{
  "scripts": {
    "build": "PROD=1 fly build",
    "build:dev": "fly build"
  }
}

Then somewhere in your build task

if (process.env.PROD) {
  // prod specific things 
}
Akkuma commented 7 years ago

I think we can do that, here is an example of what is being done elsewhere, which is what we were working on standardizing across repos

 "scripts": {
    "build": "grunt build",

    "build-help": "grunt build-help",

    "build-all": "npm run build -- --browser=all",
    "build-all-develop": "npm run build-all -- --environment=develop",
    "build-all-staging": "npm run build-all -- --environment=staging",
    "build-all-production": "npm run build-all -- --environment=production",

    "build-chrome": "npm run build -- --browser=chrome",
    "build-chrome-local": "npm run build-chrome -- --environment=local",
    "build-chrome-develop": "npm run build-chrome -- --environment=develop",
    "build-chrome-staging": "npm run build-chrome -- --environment=staging",
    "build-chrome-production": "npm run build-chrome -- --environment=production",

    "build-firefox": "npm run build -- --browser=firefox",
    "build-firefox-develop": "npm run build-firefox -- --environment=develop",
    "build-firefox-staging": "npm run build-firefox -- --environment=staging",
    "build-firefox-production": "npm run build-firefox -- --environment=production"
}
lukeed commented 7 years ago

@Akkuma Yeah, I really don't like that. I've joined teams where that was the setup & everyone sorta kept their distance from it.

By setting global vars, the most complicated of your examples becomes something like:

BRO=ff ENV=dev fly build

Merging this PR wouldn't make your scripts setup any simpler

Akkuma commented 7 years ago

That's fair enough. Part of the reason for the standardization on npm is due to different build tools, old stuff uses Grunt, my stuff uses Fly. It is simpler mentally to just use the same commands across multiple repos than remember which tool is being used.

lukeed commented 7 years ago

@Akkuma Well, you can apply the same setup to the current Grunt commands. You'd just have to adjust the internals of the build task to look for those process.env globals.