search

bdfhjk / VICER

4 stars 3 forks source link

Initial VM version + test case boilerplate #34

Closed chaser92 closed 9 years ago

rudolfkral commented 9 years ago
  1. Express is lacking in package.json - grunt's default task can't run.
  2. Please describe what you have done - or what you had in mind :) - and how do you test it in the description. It would save us some time if your reviewer wouldn't have to figure how to deal with your code all by himself :) After merging, I suggest it go to the wiki.
  3. Your "before all" hook doesn't pass the tests.

Uncaught Error: Tried loading "mod_executor/functionCall" at /xxx/VIPER/src/backend/executor/functionCall.js then tried node's require("/xxx/VIPER/src/backend/executor/./functionCall") and it failed with error: Error: Cannot find module '/xxx/VIPER/src/backend/executor/./functionCall'

Should you fix these issues, I will proceed further with the review.

rudolfkral commented 9 years ago
  1. Dumping whole VM debug to stdout by default renders test output not very helpful. You can stream those into files.
  2. I still strongly encourage simple type checks(isValue or isInt, isLocation) after popping things from the stack. It will prove especially useful during current phase of development, when wrongly built CFGs will probably happen. Prove me wrong or add it.
  3. An empty file called processor.js has been added, please insert a comment describing its purpose or delete the file.

Have a look at my comments in the code.

rudolfkral commented 9 years ago

Good job.