Skip to content

Conversation

@tomasklapka
Copy link
Contributor

@tomasklapka tomasklapka commented Sep 5, 2017

As issued in #574
This is a simple modularization of CLI so it is possible to reuse it and inject own express like this:

#!/usr/bin/env node

const startCli = require('solid-server').startCli
const express = require('express')
const rdfFormatsProxy = require('http-rdf-formats-proxy')
const server = express()

server.get('/version', function (req, res) {
  res.json({ "version": require('./package.json').version })
})
server.use('/rdf-formats-proxy', rdfFormatsProxy())

startCli(server)

EDIT: renamed cli() to startCli()

@RubenVerborgh RubenVerborgh self-requested a review September 5, 2017 14:01
Copy link
Contributor

@RubenVerborgh RubenVerborgh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with minor change suggestions.

bin/lib/cli.js Outdated

module.exports = function cli (server) {
program
.version(packageJson.version)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nitpick) Make this one line?

bin/lib/cli.js Outdated
var loadInit = require('./init')
var loadStart = require('./start')

module.exports = function cli (server) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could the function name be more descriptive? Because it doesn't return anything. Maybe startCli or something like this?

bin/lib/cli.js Outdated
var program = require('commander')
var packageJson = require('../../package.json')
var loadInit = require('./init')
var loadStart = require('./start')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps all of these can become const now.

@RubenVerborgh RubenVerborgh merged commit 310d95e into nodeSolidServer:release/v4.0.0 Sep 5, 2017
@RubenVerborgh
Copy link
Contributor

Thanks!

@tomasklapka tomasklapka deleted the modularize-cli branch September 5, 2017 20:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants