sitn / crdppf_core

Core / generic parts of the CRDPPF project
GNU General Public License v3.0
1 stars 3 forks source link

Legalfiltering #34

Closed voisardf closed 9 years ago

voisardf commented 9 years ago

@kalbermattenm: Banzai! let's give it a go ;) thanks for your review

kalbermattenm commented 9 years ago

I have done the review.

This really to big for me to do a good review... I don't really get what all the code is doing... As I already told you, you should really separate thing, like there is some stuff regarding the tiles, some stuff regarding the legal docs and so on. I also know that it was quite hard for you to split things, but if you want an efficient review, then you should do it...

Moreover, try to restrain your line length (might it be in Javascript files or Python files) to 80 characters. Otherwise, the review is really hard in GitHub. You can activate the status bar in Scite to see how many characters you have on a line.

So, I mainly checked the syntax, but I cannot review anything regarding functionality...

We should begin to implement functionnal tests... (yeah... that's quite a word...)

voisardf commented 9 years ago

addressed comments and fixed issues.

voisardf commented 9 years ago

ok for test implementations too =:0)

voisardf commented 9 years ago

merging