w3c / web-of-things-framework

208 stars 71 forks source link

Add `start:coap_demo` and `start:http_demo` in `package.json` #70

Closed hiroppy closed 8 years ago

hiroppy commented 8 years ago

Hello :)

I added start:coap_demo and start:http_demo command.

Test is failing. Is this ok?

> webofthings@0.0.1 test /Users/hoge/Programming/w3c/web-of-things-framework
> mocha

  Thing with multiple properties should initialize correctly
    setting values
      1) should have properly set values

  0 passing (10ms)
  1 failing

  1) Thing with multiple properties should initialize correctly setting values should have properly set values:
     TypeError: wot.thing is not a function
      at Context.<anonymous> (test/closure-loop-repro.js:8:17)

npm ERR! Test failed.  See above for more details.

Cheers.

zsoltpardi commented 8 years ago

That message is generated by the only test file of the project, but that file is out of date already. As you can see the test executes the wot.thing call in the framework file while the wot object has been removed from the framework file.

Unfortunately, test was never developed for the project. We discussed that it would be important to start putting tests into the project.
Either we would have to remove that failing test file or fix it. Perhaps removing that single file test is the better option, because we would need to come up with a set of proper and working tests.

hiroppy commented 8 years ago

I get you.

I won't think about test in this PR:)

Can you please check this PR?

zsoltpardi commented 8 years ago

I fully agree that we should remove the "start": "node demo.js" line since the demo.js file was deleted, as well as we should remove the "test": "./node_modules/.bin/mocha" line as the test is incorrect, but I think we should not introduce the following lines in the PR:

start": "npm run start:coap_demo & npm run start:http_demo",
"start:coap_demo": "node ./examples/coap_demo/demo.js",
"start:http_demo": "node ./examples/http_demo/demo.js",

In my opinion these are domain specific start parameters - some users might want to run other demos than the http or coap. I am not sure what is the best practice in this case, but I think, if a user really must use npm to start the program then the package json should be modified locally to start the desired script instead of hard code such start parameters in the master branch. Please let me know what you think.

hiroppy commented 8 years ago

I understood to hear your opinion. so I return this line.

hiroppy commented 8 years ago

I created new PR because this branch name is incorrect :) https://github.com/w3c/web-of-things-framework/pull/72