mozilla / node-firefox

node.js modules for interacting with Firefox via the DevTools remote protocol
https://www.npmjs.org/package/firefox
Mozilla Public License 2.0
301 stars 18 forks source link

Module guidelines #7

Closed sole closed 9 years ago

sole commented 9 years ago

I'm thinking of adding these guidelines to the README as a new section so we are all on the same page. It's a way of formalising/dumping the knowledge we've acquired while tidying up the first two modules. Did I forget anything? @tofumatt @brittanystoroz

1) remove as many dependencies as possible and make the code super simple. Right now it is using underscore and Q and the code looks too cryptic unless you're familiar with those. So far using es6-promise and Promise.all has got me covered.

2) Try to be as obvious as possible. This means, for example, not using "B2G" if we can say "simulator" instead. Another example from this module: startB2G can be renamed to startSimulator and it is so much evident what it is doing for someone who is not familiar with the moz-jargon.

3) Only expose the API via promises. Granted you internally might need to use callbacks for other modules which are not promisified, but we want to make the whole API follow the same style, so once you learn to use one module, using another node-firefox- module is easy because the interface is the same: makeACall().then(doSomethingElse).

4) No CLI binary. None of the existing examples/use cases we came up with were using the CLI, so there's either no need for that or we hate the CLI. Either way there was a lot of duplicated code between modules for doing CLI and it wasn't too consistent, and we do not have too much time for this first iteration, so no CLI for now.

5) For naming, try to be as generic as possible but also as semantic as possible. For example, if a module can be used to discover firefox and firefox OS runtimes, try to avoid adding 'os' to its name, so that it is more generic. Also consider if something sounds weird, maybe it's because we're using the wrong name. E.g. fx-simulators sounded weird until it was renamed to find-simulators. When in doubt, discuss it with more people! Don't be afraid :D (specially if English is not your first language!)

6) Use the build-tools module and gulpfile.js to lint the code.

MattiSG commented 9 years ago

:+1: guidelines are very important. I'd advise putting them in a CONTRIBUTING.md file rather than README.md.

This has two advantages:

  1. Keep the user doc separate from contributor doc, avoid overloading README.
  2. GitHub will add a "Read the guidelines for contributing first" in any PR sent to this repo :)

Matti Schneider

Sent from a phone, please excuse the terseness.

Le 15 janv. 2015 à 14:14, sole notifications@github.com a écrit :

I'm thinking of adding these guidelines to the README as a new section so we are all on the same page. It's a way of formalising/dumping the knowledge we've acquired while tidying up the first two modules. Did I forget anything? @tofumatt @brittanystoroz

1) remove as many dependencies as possible and make the code super simple. Right now it is using underscore and Q and the code looks too cryptic unless you're familiar with those. So far using es6-promise and Promise.all has got me covered.

2) Try to be as obvious as possible. This means, for example, not using "B2G" if we can say "simulator" instead. Another example from this module: startB2G can be renamed to startSimulator and it is so much evident what it is doing for someone who is not familiar with the moz-jargon.

3) Only expose the API via promises. Granted you internally might need to use callbacks for other modules which are not promisified, but we want to make the whole API follow the same style, so once you learn to use one module, using another node-firefox- module is easy because the interface is the same: makeACall().then(doSomethingElse).

4) No CLI binary. None of the existing examples/use cases we came up with were using the CLI, so there's either no need for that or we hate the CLI. Either way there was a lot of duplicated code between modules for doing CLI and it wasn't too consistent, and we do not have too much time for this first iteration, so no CLI for now.

5) For naming, try to be as generic as possible but also as semantic as possible. For example, if a module can be used to discover firefox and firefox OS runtimes, try to avoid adding 'os' to its name, so that it is more generic. Also consider if something sounds weird, maybe it's because we're using the wrong name. E.g. fx-simulators sounded weird until it was renamed to find-simulators. When in doubt, discuss it with more people! Don't be afraid :D (specially if English is not your first language!)

— Reply to this email directly or view it on GitHub.

brittanydionigi commented 9 years ago

A+ and +1 to @MattiSG for outlining this in a contributing file

tofumatt commented 9 years ago

I guess the short version is I'm all for this and also agree that a lot of it belongs in a contributing file. Yay :+1:

sole commented 9 years ago

Resolved via https://github.com/mozilla/node-firefox/pull/8