Overall project discussion

[satazor]

This thread serves as an open discussion to features, implementation details and misc stuff related to the CLI project.

I will keep the section below updated to what we have agreed upon.

TODO

[satazor]

I usually use the following modules to create CLI's:

• yargs for arguments processing and help usage
• update-notifier to notify CLI updates

Answering some of your points in npms-io/npms-api#23 (comment)

• linting rules: eslint + @satazor/eslint-config
• test runner: mocha
• assertion style: chai expect
• workflow: gitflow
• test structure: it really depends, but I usually do multiple expects in the same test
• BDD

What are your thoughts?

[satazor]

Regarding creating a SDK layer, I don't really know.. It's one more piece of software to maintain and it may leak the abstraction. For instance, if the search response changes from a flat list to a nested list, you might want to return the new nested list instead of the flat list, e.g.:

[{ name: 'jquery', version: '2.0.0', score: .., searchScore: ... }]  // flat list
[{ module: { name: 'jquery', version: '2.0.0'}, score: ...,  searchScore: ...}] // nested list

Not doing so, will introduce discrepancies between the API and the SDK which will lead to confusion. Of course we could have proper versioning of the SDK but, without resorting to deprecation, we would need to maintain a lot of output variations for the endpoints.

[dcrockwell]

SDK can mean a lot of things, of course from a.) A full-blown Object Oriented Data Abstraction Layer, to z.) A simple communications abstraction for speaking with the server.

I am proposing the latter: a simple communications abstraction so that when you make changes to the server's request/response pattern, you can gracefully issue a patch and even eventually support legacy API versions for applications that integrate early.

[dcrockwell]

discrepancies between the API and the SDK

If you make the SDK the driver, then this cannot happen. Feature changes are proposed in the SDK first, then subsequent proposals are made for both the client and server to match the SDK.

This works because the SDK is the practical interface to the system for both your own client, and everybody else's.

[satazor]

@dcrockwell the thing is that not everyone will be using the SDK, especially the ones using other languages other than JS.

[satazor]

We will need to support every single change we make to the API, so that we do not break the SDK output. This will easily become very tedious :s. I prefer to stabilize the API and document it and then apply versioning to the API itself similar to what other services like twitter do. Though, I doubt it will ever be necessary.

Regarding the API endpoints that the CLI will need, I will give priority to those so that the CLI doesn't need to be updated later. Regarding #1 (comment), what are your thoughts?

[dcrockwell]

I'm thinking in terms of the CLI.

Writing a CLI against raw REST requests will still require an object to centralize all of the calls, or it will have decentralized remote calls spaghetti'd throughout which is not ideal for workflow (especially if you're expecting changes).

If an object needs to be written to perform the REST requests anyways, doesn't it make sense to modularize that component so that it can be used as an easy form of integration for anybody using the primary language this search serves?

Otherwise, we just have an object we still have to maintain, but it's not useful outside of the CLI.

[satazor]

@dcrockwell I agree with you as long as we don't have to be backwards compatible in terms of the responses. This means that the SDK will be a simpler way to consume the API. Everytime the response is incompatible we need to bump the major version and deprecated the old one.

[satazor]

Created: https://github.com/npms-io/npms-api-client

[dcrockwell]

@satazor Sounds like a good initial plan.

Perhaps "npms-node", so that "npms-python" and more could be made in the future?

It may be a bit early to consider it, but looking into automating this layer with something like Swagger.io may be beneficial:

• it can be automatically generated the SDK in any language the project wants
• there's a single place to make changes for all languages generated, instead of maintaining many repos

[dcrockwell]

linting rules: eslint + @satazor/eslint-config
test runner: mocha
assertion style: chai expect
workflow: gitflow
test structure: it really depends, but I usually do multiple expects in the same test
BDD

Sounds good for everything except I suggest switching to a single assertion per test. It's only slightly more verbose and provides a standard for assertions. When there are more than one assertion in a single test, it can sometimes lead to ambiguous results on summaries, and extra work for the developer to figure out which assertion is failing.

[satazor]

Perhaps "npms-node", so that "npms-python" and more could be made in the future?

It may be a bit early to consider it, but looking into automating this layer with something like Swagger.io may be beneficial:

• it can be automatically generated the SDK in any language the project wants
• there's a single place to make changes for all languages generated, instead of maintaining many repos

I have never used swagger, did you? Do you think it's good? My only concern is if we are overcomplicating things.

[satazor]

Sounds good for everything except I suggest switching to a single assertion per test. It's only slightly more verbose and provides a standard for assertions. When there are more than one assertion in a single test, it can sometimes lead to ambiguous results on summaries, and extra work for the developer to figure out which assertion is failing.

SGTM, the tests for the CLI will be simple. Regarding coverage, I use istanbul and coveralls.

[dcrockwell]

I have not used Swagger or anything like it, really. I've just heard some rumors about it being good for this sort of thing.

I'm 100% with you on taking the Amish approach to technological adoption: if it makes things convenient at the expense of overly complicating another aspect of life, it should be rejected.

If automating this layer is more trouble than it's worth, it should be disregarded, but perhaps it's worth an hour of research to know that for sure.

[satazor]

If automating this layer is more trouble than it's worth, it should be disregarded, but perhaps it's worth an hour of research to know that for sure.

Alright, will you try it out and report back?

[dcrockwell]

SGTM, the tests for the CLI will be simple. Regarding coverage, I use istanbul and coveralls.

Perfect. We have an identical testing stack.

I think 100% coverage is the only coverage worth going for, unless you can identify with certainty that only specific key parts warrant testing, and the other specific parts do not.

Plus, "86.2%" coverage doesn't have much meaning to anybody except to say that the project is not fully covered.

[dcrockwell]

That said; there are certain aspects of testing a CLI which call into question what constitutes 100% coverage, as istanbul cannot instrument the CLI code from what I can see.

[satazor]

That said, there are certain aspects of testing a CLI which call into question what constitutes 100% coverage, as istanbul cannot instrument the CLI code from what I can see.

You are right, but if the CLI bin is simply enough to call functions based on the commands, perhaps it doesn't need to be covered. If you still need to cover it, I think there was a module that does what you want, and I think that AVA uses it.

[dcrockwell]

Yeah, this CLI will be simpler than others. It won't require much more than some basic integration testing for: help(-h,--help), version (-V,--version), that errors passed from the library are thrown, and that each function has basic coverage against errors.

[rhettl]

Any thoughts on commander over yargs?
A good argument in favor of the SDK is that it's easier to unit test the SDK then E2E test the CLI with some unit tests here and there.

[satazor]

I have had so many troubles with commander in the past that I've totally gave up..I can't remember them exactly but they were so frustrating..

[dcrockwell]

Any thoughts on commander over yargs?

commander has some advantages when creating complex "git-like" interfaces, but ends up being high-calorie sugar for super-simple CLIs like I presume this one to be.

(@satazor perhaps defining what the CLI features will be can be the next step.)

it's easier to unit test the SDK then E2E test the CLI with some unit tests here and there.

In my experience, CLI testing is black-boxed, so the choice of CLI library is inconsequential to testing.. though I'm eager to be proved wrong on that as testing a CLI right now kind of sucks.

tj/commander.js#438

[satazor]

@dcrockwell I see two essential features for launch:

• search of modules
• get collected, evaluation and score info of a given module

We don't need to do any offline support for now. We could have an --output/--json option to be able to specify JSON output besides the human readable one.

[dcrockwell]

@satazor Excellent. Let's begin nailing down exactly how we envision it looking from the CLI. Does this look about right to you as a first stake in the ground?

Basic Search:

$ npms search mrt

Searching {search url here}..
1 package found!

[email protected]

Partial Search:

$ npms search mr

Searching {search url here}..
3 packages found!

[email protected]
[email protected]
[email protected]

JSON output:

Actually this section poses an issue. What format will the JSON take, and what's best for future growth?

I personally think this may be a job for http://jsonapi.org/. It has built-in considerations for relationships, meta-data, and pagination.

Any standard like it would work, but it's the only one I know of so far.

$ npms search mrt --json
{
    ???
}

Opt-Out:

Not sure how this would be handled.

[satazor]

Whats the difference between basic and partial search?

$ npms search jquery

Searching {search url here}..
667 packages found!

[email protected] <score> <repo>  (we could make this tabular and add more info?)
...

$ npms search jquery --json

[
  { "name": "jquery", version: "2.0.0", ... },
  ...
]

$ npms info jquery

(pretty output with colors)
{
   collected: { metadata: {}, github: {}, ... },
   evaluation: { ... },
   score: { ... }
}

$ npms info jquery --json

{
   "collected": { "metadata": { ... }, "github": { ... }, ... },
   "evaluation": { ... },
   "score": { ... }
}

Regarding the --json option: when you are rendering a template, you specify an object which contains the data to be interpolated. The --json option spits that data directly.. thats how I envision it. The json output is extremely helpful to allow this CLI to be easily consumed by other automated software.

[dcrockwell]

Whats the difference between basic and partial search?

Nothing. Just two examples to show full and partial searching with the same interface without the need for wildcards.

The --json option spits that data directly

I think there's a need for both a human-readable and machine-readable interface, and that JSON does not fit both use cases well.

So, I would prefer something similar to this:

$ npms info jquery

[email protected]
Score: 2.4
Evaluation:
    coverage: 100%
    lines: 268
    size: 10kb

$ npms info jquery --format=json
{
   collected: { metadata: {}, github: {}, ... },
   evaluation: {},
   score: {}
}

[dcrockwell]

Perhaps the concept of reporters should be built in from the get-go?

[satazor]

@dcrockwell yes, that was an example.. we could use something like https://www.npmjs.com/package/print-object or https://www.npmjs.com/package/purdy for the human readable output of npms info

[satazor]

Perhaps the concept of reporters should be built in from the get-go?

@dcrockwell as long as it is simple.. I don't want to complicate things too much

[satazor]

Yes, it looks more suitable to HTTP API's because of the links and stuff.. I don't think we need it..

[flesch]

Hey there! I put together an npms CLI – it doesn't meet all the requirements above (I wrote most of it before finding this thread), but thought you'd like to see it:

https://github.com/flesch/npms-cli

I did push it to npm, but I'm not intending to step on any toes, so I'm happy to change the name or even transfer ownership to your org.

[satazor]

@flesch That looks good!

One minor thing I would change: since we expect to have more commands, such as info, the bin/npms.js will become very complex. Would you mind refactoring the search code into lib/search.js?

Could you also add eslint and editorconfig to the project?

@dcrockwell what do you think of @flesch work?

[mikeerickson]

@flesch @satazor I am glad I stumbled upon this thread before the weekend hit as I was planning on creating a CLI as well. Now I can work on something else :-)

[flesch]

@satazor Awesome - I will definitely make those changes! @mikeerickson is going to add some tests too (https://github.com/flesch/npms-cli/issues/3).

[dcrockwell]

Hey guys, I've been moving to another city for days.

I think @flesch's work is good, but it's my opinion that we stick to our guns and do it 100% test-driven from the beginning with continuous integration. It could be good to have something now, but that will result in a lot of people needing to rework their integrations.

I also think that npms-cli ought to be the official package name, so @flesch ought to transfer ownership to npms-io before any more versioned publishes are made (which will have to be unpublished, and can't be re-used).

Seeing as we're in a pre-alpha stage now, we should start on version 0.0.0 until we have an alpha to publish as 0.0.x. From there, the first beta release would be 0.1.0, and then the official first gold release be 1.0.1 (as 1.0.0 can not be used again).

Testing-first is a requirement for my participation on a project.

[dcrockwell]

Since it seems there are more than a few of us who want this thing and are willing to do some work to get the ball rolling, I'd like to propose the following next steps:

1. Stub an initial spec. No working test code. Just behavior stubs.
2. Approve the stubs.

After the stubs are approved, we can break it up by feature and delegate the work out to multiple devs.

We can use git flow features to make sure we don't step on each other's feet.

[dcrockwell]

I also think that if @flesch is cool with it, we could copy and paste his code into our features so that they make our tests pass with a little modification. This way his work is still utilized.

[mikeerickson]

@dcrockwell Sounds like you have it all planned, have fun!

[dcrockwell]

It's really all up to @satazor to call the shots. I'm only offering suggestions to try and keep the original project requirements met as we've been working out for a while, but I've been away for many days from this discussion so @satazor's ideas for the project may have changed.

[mikeerickson]

@dcrockwell You requirements are your requirements, I am not going to object. I have plenty of projects to work on. If this project is open for contributions, I will reevaluate at that time. There was progress made this well and I (and @flesch) were stepping in to try and help.

But you have other plans, so I will step back

[satazor]

This is a community project, so lets join forces and work this out together. @flesch's work is a good start but I must agree that some goals were left behind:

• Use update-notifier
• Support JSON output with --json
• Create an SDK to consume the API, and use it in the npms-cli project
• eslint + editorconfig
• Tests

I want this project to be easy to maintain and to modify, so tests are a must. I would really love to have all of you work on this together, so please do not fight each other. I'm sure we can setup some common ground.

Having that said, I suggest that we follow our initial goals and use @flesch work in the process.

@dcrockwell When are you able to use some of your time on planning+creating the stubs?

[dcrockwell]

@satazor I can make the initial stubs tonight and submit them to this repo as a PR. Then the PR could be used as discussion for accepting/rejecting/modifying the proposed stubs

[dcrockwell]

@satazor I can't fork the repo until at least a README.md is committed, but I can still work on the stubs.

[satazor]

@dcrockwell done.

[dcrockwell]

I'm going to need a couple more days to finish up the stubs. Lots of work on my desk ATM.

[flesch]

I've added @satazor and @dcrockwell to the owners of https://www.npmjs.com/package/npms-cli so you guys can publish when you're ready. I've also deprecated v1.0.0.

Do you mind if I publish one more release to work with the new API format? The latest response format broke my version, leaving a few users hanging.

[dcrockwell]

@flesch I'm just a contributor to the project and not an owner. @satazor should be the sole owner.

That said, in my personal opinion I think if you publish another version, it should probably be 0.0.1 to signify it as an alpha.

[satazor]

@flesch go ahead and push 1.0.1

[mikeerickson]

@satazor @flesch I will be submitting PR for the fix Saturday, I will post here when completed as well

[mikeerickson]

@satazor @flesch All done here https://github.com/flesch/npms-cli/pull/6 Updated version to 1.0.1 as outlined above.

[flesch]

I just pushed v1.0.1 to npm - it supports the new API format (thanks @mikeerickson!). I also did refactor the code to move the "search" command into its own file. It's a start towards some of the requirements above, but let me know how I can continue to help!

[satazor]

Since @dcrockwell appears to be overwhelmed with work I propose that we embrace @flesch work into the official repository. If you agree @flesch I will be giving you access to the cli repository so that you can push your work into it.

• push -f into this repository
• Remove warning notice from the README
• Add eslint config to the repository, preferably using the same eslint config from other npms repositories. Update code to make sure that code is validated; you may also add lint to the scripts, see: https://github.com/npms-io/npms-analyzer/blob/master/package.json#L7
• Add devDependencies badge to README
• Create an issue in the repository to add tests
• Update LICENSE, see: npms-io/npms-www@66cf251

After migrating to the npms repository can you make the above changes @flesch?

Just a reminder to myself: when all of this is done, enable greenkeeper and travis for this repository - need to add a dummy test for now.

[dcrockwell]

@satazor Sounds like a good plan. I'm very behind on a few repos and thought I would be done by now. Apologies for affecting your release schedule.

[flesch]

👍 I'll try to make these changes soon!

[flesch]

@satazor I just pushed a couple of the quicker changes. I'll work on getting linting in as soon as I can. eslint is in too!

[satazor]

@flesch Thank you very much for your hard work <3

[satazor]

I'm going to close this issue since the discussion is pretty much done now. Thanks for all thr contributions you guys made.