added node version check and updated the licence to a correct string

[gorhgorh]

Hi there, this commit add a section to the package.json where we can define a node version range that is supported by the CLI and a check in the bin/adapt that verifies that the user node version is matching it.
Inputs are welcome on what version are supported. (currently it is limited to 0.10.33)

It also add a valid string for the licence. (GPL-3.0)

[oliverfoster]

+1 beautiful - i'm on v0.10.38 actually, now i'm looking

$ node --version

[moloko]

+1

[tomgreenfield]

+1

(I'd consider allowing up to v0.10.40. I know myself and a few others are on higher patch versions than .33)

[gorhgorh]

thanks for the input @tomgreenfield i just need to learn how to update a pr properly and resubmit :)

[gorhgorh]

version range updated to 0.10.40

[oliverfoster]

@brian-learningpool @cajones @chucklorenz @lc-thomasberger @taylortom @dancgray @darylhedley @hrajotia have ya'll got any different node versions running adapt-cli other than 0.10.33 > 0.10.40 ?

[brian-learningpool]

@oliverfoster, I believe before he went on holiday that @dennis-learningpool had successfully installed the framework and authoring tool with Node v4. Perhaps the upper version range shouldn't be as restricted?

[oliverfoster]

@brian-learningpool - do you know if that is so far restricted to dennis only or do we have people using node 4 in the wild? i'd quite like us to know which versions of node we're good to use.

[brian-learningpool]

I'm not sure, Ollie. The documentation on the authoring tool still recommends 0.10.33. I think Dennis was just curious about installing Node v4 and checked how Adapt ran on it.

[oliverfoster]

ok. so can we leave it at v0.10 as the current upper limit, then verify that v4 is working and bump the upper limit, documentation and announce in the forum when we're all happy? can use dennis' test as a platform to do that. i'll add a jira ticket for node v4 support if so, as it's an environment for both the AT and Framework both camps will need to been satisfied.

[brian-learningpool]

-1
I like the code in principle, but 0.10 is several versions behind now. Also, what if users have 0.12 installed (as I suspect there are) and the adapt_cli works without issue? If this PR goes in all of a sudden they'll start getting an error about their node version.

I think only the authoring tool was explicit about the Node version required. What I'm getting at is that we need a more considered approach to the Node environment we're going to support. Right now it's all a bit vague, and I think merging this PR will cause people some problems.

[oliverfoster]

fair points, how about switching the error to a warning? and proceeding as described?

[brian-learningpool]

@oliverfoster, I'd prefer us to wait and test with the LTS supported version of NodeJS, i.e. 4.2 (which is due out soon) and standardise the environment requirement across all our core repos.

[oliverfoster]

ok. when is that happening and do you need testers?

[brian-learningpool]

I think Node 4.2.0 is due out within the next two weeks, according to this.

We would require testers but developers could move to Node 4.x for our next sprints in the meantime. Let's discuss it on the call on Monday.

[oliverfoster]

Yep, sounds good. I've been thinking it'd probably be less restrictive to change the error to a nice big red warning : "the version of node you are using has not been tested with adapt etc, please use v x or help us to test this version properly by following the directions at xxx" ?

[moloko]

sounds good to me

[brian-learningpool]

@gorhgorh, can you convert the error to a warning as @oliverfoster has suggested?

We'll review this when we verify Node 4.2 works with all our dependencies.

[oliverfoster]

Something like:

var chalk = require("chalk");
var errString = 'adapt-cli has been tested on node versions between '+pkg.engines.node+', you are curently using ' + curVer + ' - errors may occur.';

if(semver.satisfies(curVer, pkg.engines.node) === false){
  console.log(chalk.red(errString));
}

Please change the wording. English is not my first language.

[oliverfoster]

+1

[gorhgorh]

I start to have a lot of commit for a simple change, do you know if I can rebase safely to have only one commit ?

[gorhgorh]

squashed and ready =)

[moloko]

+1
@oliverfoster Javascript is your first language isn't it?

[oliverfoster]

+1
qbasic and dog/cow/sheep noises

[gorhgorh]

it does not seems to be usefull anymore and I don't like to see unmerged prs =)

[oliverfoster]

this is still pretty useful, helps police the boundaries of usability and support

[gorhgorh]

ahh ok , i belived that now the framework is tested again latest version of node, did i dreamt it ?