search

hubot-archive / hubot-auth

Assign roles to users and restrict command access in other scripts
149 stars 54 forks source link

Updated tests to use hubot-test-helper for test clarity #29

Closed cecilia-sanare closed 8 years ago

cecilia-sanare commented 8 years ago

Also updated various packages to their latest version

This shaved off a few lines of code, however the big improvement in my mind is getting rid of the ambiguous message skipping you had to do with the adapters reply event.

patcon commented 8 years ago

cool! thoughts on resolving those failures?

cecilia-sanare commented 8 years ago

Seems to be related to Node 0.10 not supporting promises.

I'm going to change the Node versions to latest and lts that way we cover the versions people are likely to install.

Side Note: Node v0.11 worked, but v0.10 didn't.

cecilia-sanare commented 8 years ago

Resolved the issue with the old node versions being used with Travis. Also squashed all of my commits into a single commit.

patcon commented 8 years ago

Thanks so much Cecilia, but the rebase actually makes this more difficult to actually review...! In the future, please either 1) not squash changes that include unnecessary style changes beyond the PR scope, or 2) not include style changes in with functional changes

I'll on the go right now, but will review later :) If you still have the reflog, and it's simple to revert and push, that would probably resolve the above concern. thanks!

cecilia-sanare commented 8 years ago

Sorry about that, reverted the squash.

patcon commented 8 years ago

Ugh. I'm an idiot. Sorry man. I didn't look closely enough and so didn't realize you'd totally rewritten. (which is awesome)

Thanks for reverting even though it wasn't a sensible request :)