Talk:Service-template-node/GettingStarted

The coding style of the service nodes is different from that in MediaWiki core. Given it's confusing to switch between coding styles I'd recommend adding eslint or jscs to the service template which will enforce and autofix a lot of these.

See https://gerrit.wikimedia.org/r/#/c/320802/ - we could have avoided this exchange altogether and saved time had it been in the repo!

During code review

https://gerrit.wikimedia.org/r/#/c/318507/3/routes/v1.js

I was told

"It would be more future-proof to actually delete this file and create a new one for your endpoints and call it something like v1/trending-v1.js"

Can I suggest a comment is added at the top of the file to make it clear that this file should be deleted and/or should not be edited by forked projects?

The service template node includes various routes and tests for them including info.js

This is a little confusing as it mixes examples with dependencies.

It seems more intuitive for these routes to live in their own repo and be installed via `npm install` rather than bundled in the repo.

When cloning the repo I removed these as I thought they were examples. Having these as dependencies would make the repo easier to grok and prevent making mistakes like this: https://gerrit.wikimedia.org/r/#/c/318507/3/test/features/info/info.js,unified

I should also add that I removed this as the tests were not passing and it was not clear how to fix them from the README. Given I thought it was just an example route I took the easy option of removing it.

1) service information should redirect to the service home page:

HTTPError: 404

at Request.P.try.bind.then.err.retry.HTTPError.status (node_modules/preq/index.js:232:23)

at Request.tryCatcher (node_modules/bluebird/js/release/util.js:16:23)

at Promise._settlePromiseFromHandler (node_modules/bluebird/js/release/promise.js:510:31)

at Promise._settlePromise (node_modules/bluebird/js/release/promise.js:567:18)

at Promise._settlePromise0 (node_modules/bluebird/js/release/promise.js:612:10)

at Promise._settlePromises (node_modules/bluebird/js/release/promise.js:691:18)

at Promise._fulfill (node_modules/bluebird/js/release/promise.js:636:18)

at Request._callback (node_modules/bluebird/js/release/nodeback.js:45:21)

at Request.self.callback (node_modules/request/request.js:186:22)

at Request.<anonymous> (node_modules/request/request.js:1081:10)

at IncomingMessage.<anonymous> (node_modules/request/request.js:1001:12)

at endReadableNT (_stream_readable.js:926:12)

at _combinedTickCallback (internal/process/next_tick.js:74:11)

at process._tickCallback (internal/process/next_tick.js:98:9)

2) service information should get the service info:

AssertionError: No home field returned!

A newbie (read me :-)) cloning the service template node may not realise the significance/importance of spec.template.yaml right away. It is also very large so it's not clear if it should be appended to/emptied or replaced.

I'd suggest at the very least a note is made in the README file guiding the user about what to do with this e.g. create a file spec.yaml

The fact it contains lots of pre-existing routes such as `/ex/err/manual/deny`is also confusing - it's not clear whether these are examples or ones that need to be included.

If the routes are expected to remain in the repo I'd suggest renaming this to spec.yaml in the template node itself to make that clearer (I'm still not clear on that)

The package.json has a lot of fields that are not necessary right away which can slow down getting started.

After cloning the repo, it was pointed out to me that I already had erroneously assigned a version, keywords, author, homepage, repository and package description.

It shouldn't be necessary so early on to have to worry about these fields. Developers have different workflows and some (myself included) like to think about these prior to deployment.

Any node developer knows what these are for and having them in the template node is just irritating and can lead to erroneous information.

I assume these are important as part of deployment but it seems worrying about these can be delayed until the user wants to deploy their repo.