search

LuminosoInsight / sales-engineering-code

Code for sales engineering, particularly for code that will be given to customers
MIT License
0 stars 0 forks source link

Added deduper #12

Closed tobrien6 closed 9 years ago

slibs63 commented 9 years ago

The whole script could benefit a bit from some PEP-8 attention, but that's probably less important considering that it's just sales engineering code.

tobrien6 commented 9 years ago

Actually if you could let me know what the glaring PEP8 violations are, I'd appreciate that. I haven't read through it closely.

slibs63 commented 9 years ago

The most glaring PEP-8 issue is the 80-characters per line limit, which some people feel more strongly about than others. Next, we usually put spaces between #s and the starts of comments. Also, spaces between operators and operands (e.g. x[0] + lower is better than x[0]+lower, s[0] != s[1] is better than s[0]!=s[1], etc.)

slibs63 commented 9 years ago

Last big-picture thing, perhaps a slight reorganization of the methods could help make this more readable. I'd put helper functions like intervals and chunks closer to the top, grouped more or less by what they do, with the meatier functions further down.

tobrien6 commented 9 years ago

Should I merge now?

slibs63 commented 9 years ago

I'm just going to give it one more pass to see if I spot anything else. I'll merge it once any additional agreed-upon changes have been made.