carrot / ship

⛔️ currently unmaintained ⛔️
Other
151 stars 14 forks source link

use single interface for everything #32

Closed notslang closed 10 years ago

notslang commented 10 years ago

ok, I should have spent more time thinking before I implemented that shipfile class. that was a terrible idea - it means that there are 2 entirely separate ways of controlling ship: the CLI and the config file. And these separate interfaces both represent pretty much the same thing, so it's stupid to treat them differently and had I continued down that path I probably would have ended up writing a whole bunch of logic to mash the CLI args and config file together.

So, I've decided to sorta get rid of the idea of a "config file" and do everything through a well designed CLI. However, you may notice that there are a lot of args needed to do a deploy - you are right, and yes: that would get annoying to type every time you need to deploy. So, I'm borrowing an idea from mocha (and a few other programs): using .opt files. The opt files are like config files, and you can choose which one to load (defaulting to ./ship.opt) depending on the deploy you want to do. But inside the opts file is just a list of CLI args that get appended to the end of the command.

This way

kylemac commented 10 years ago

@slang800 I don't agree with this direction. In fact, I still believe in our original abstraction of ship.conf. I recognize the motive of wanting to share the logic, but I think that comes at the expense of our user's interface. And a clear interface is one of, if not the biggest motivating aspects of the ship project. In my opinion the cli-arg pattern (load files) is bit too low-level for the kind of convenience we are offering our users.

I'm also closing this because I don't think we should be debating a new interface change, since this would be our third or fourth probably. Rather we need to make hard progress and stop spending too much time in the theoretical.

notslang commented 10 years ago

That's rather surprising... I thought you'd love it.

as for the user interface, opt files look something like this:

--deployer s3
--access-key 'xxxx'
--secret-key 'xxxx'

...which has been proven to work by mocha. Whereas a json file looks like this:

{
  "deployer": "s3"
  "accessKey": "xxxx"
  "secretKey": "xxxx"
}

I think that's an improvement.

As for the new interface change: I'm sorry that I didn't think of this sooner, but the interface is very important to get right. Also, all the logic for this change is already written:

try {
  var opts = fs.readFileSync('ship.opts', 'utf8')
    .trim()
    .split(/\s+/);
  process.argv = process.argv
    .slice(0, 2)
    .concat(opts.concat(process.argv.slice(2)));
} catch (e) {}

...and this pattern would reduce the amount of logic in this project and bring us closer to a stable release.

jescalan commented 10 years ago

I think our plan was to have ship come with a great javascript API, so it can be interacted with through javascript or through the cli. Under that assumption, I think it's a little easier to be ok with the ship.conf file. Really, the CLI is just a way to collect configuration that is passed through to the javascript api, and in this case, rather than allowing you to select all the options through flags and positional arguments and such, the CLI looks for the config file and loads it into the js api directly instead. And as an extra convenience, as we've discussed in other issues, if you don't have a config file, it would prompt you for the values and create one for you.

This actually does use a single interface for everything. It's not a super standard CLI, but it should work nicely for this use case and offer a number of advantages:

That being said, I do agree with Kyle that it's much more important to us right now to actually get this working than to continually tinker with the interface. We have a functional interface at the moment, what we need is more working and tested deployers. Even if we do decide the change the interface later, which we absolutely could do, the deployers will still be the same, so any progress we make on deployers is super solid progress, whereas fiddling with this interface, while of course it can be useful to improve and discuss any aspect of a project, is just not what we need at the moment. The most important piece to shipping things (hah) is to get them out the door. Right now, it's not the interface that's blocking us from that, it's the lack of solid deployers.

notslang commented 10 years ago

The JS API is separate and unrelated to the config file and the CLI (argparse acts as an abstraction and the API just takes a hash of options, it doesn't touch the config file).

And I don't think that the interface is functional... it was broken when working on #29.

kylemac commented 10 years ago

This isn't up for any more debate, team. We will be going with the ship.conf interface.