lunarmodules / luacov

LuaCov is a simple coverage analyzer for Lua code.
http://lunarmodules.github.io/luacov/
MIT License
300 stars 68 forks source link

Change. Implement object model for reporter. #7

Closed moteus closed 10 years ago

hishamhm commented 10 years ago

Thanks for the patch!

Code review:

moteus commented 10 years ago

There's a stray local l l

:) Original code has error. It creates new local equals variable.

Also I think may be we should use config to select reporter reporter = 'coveralls'.

hishamhm commented 10 years ago

Understood. In any case local x x,y in a single line is a very misleading idiom!

moteus commented 10 years ago

This version has some unsolved problems:

1) I think we should add comman line switch/config option to select reporter (luacov -r coveralls pop3)

2) Coveralls reporter has problem with convertion file paths to repo related paths. E.g. this is example of convert in my lua-path module

/usr/local/share/lua/5.1/path.lua   =>  lua/path.lua
/usr/local/share/lua/5.1/path/findfile.lua  =>  lua/path/findfile.lua
/usr/local/share/lua/5.1/path/lfs/impl/fs.lua   =>  lua/path/lfs/impl/fs.lua

I do not think this problem has general solution so we need specify it in config file.

hishamhm commented 10 years ago

Merged. I think luacov.coveralls should be distributed as a separate rockspec, so that it loads as an add-on to luacov. This will isolate the json dependency from the core.

moteus commented 10 years ago

I create luacov-coveralls repo.

Also current version of luacov does not use .luacov as default config. Could you please open issuses for this repo?