sroberts / hubot-vtr-scripts

Scripts for making Hubot a CND Sidekick
MIT License
59 stars 15 forks source link

Adds simple mac address lookup #27

Closed patcon closed 10 years ago

patcon commented 10 years ago

No tests as I haven't worked through hubot script testing yet, but I'll give it a shot later

mattjay commented 10 years ago

Don't know much about the dependency but looking at how its implemented it seems like it could be achieved without it. I'm not opposed either way just thinking out loud. Also, don't know enough coffeescript to say that my feeling is actually possible.

mattjay commented 10 years ago

@patcon thanks for the PR thought! really cool feature idea. I think we are moving towards this project getting much larger so I think it might be a good idea (i'll see if @sroberts or @technoskald agree) that we make it a requirement to include at least some sort of unit test in a PR.

Would you be comfortable taking a stab at one? Check out the spec/ folder for some examples.

mattjay commented 10 years ago

I'm just now seeing the "no tests" comment at the top and feel dumb. I'm fine with this PR as is in that case @sroberts. We can iterate later and improve by adding tests or refactoring code. I'm all for just getting working code in the lib.

krmaxwell commented 10 years ago

I am not sure we're ready to require unit tests yet anyway, since I have no idea how to do them...

sroberts commented 10 years ago

@technoskald get on my level bro!

Naa, not my level either. I do need to learn those. Maybe I'll have to con @mattjay to write a quick guide to building tests for this.

Anyway, I'm good to go on this PR, so I'm gonna merge!

mattjay commented 10 years ago

:+1: good stuff. Thanks @patcon !!

patcon commented 10 years ago

haha thanks gents. I'll get better. I promise. INSPIRING ANIMAL IMAGERY!

Animals getting shit done on the interflag.