New .env configs and possibly some bugs fixed

jamiew / helium-discord-bot

Helium hotspot stats & leaderboards for your Discord server

13 stars 3 forks source link

New .env configs and possibly some bugs fixed #20

Closed ThaMunsta closed 3 years ago

ThaMunsta commented 3 years ago

Added max activity config in the .env file!
Bug fix with finding spaces in hotspot names - looks like you have your own branch refactoring this so ignore anything I'm duplicating
The if undefined logic was flipped but that doesn't seem to work right for me. When it's not undefined we should have a payload to work with?

jamiew commented 3 years ago

Nice patches. PR got a little wonky since I think you branched from your original branch; are you comfortable either trying full rebase (might be messy) or making a fresh branch and cherry-picking your new commits? I usually just do the latter since I hate messy rebases

ThaMunsta commented 3 years ago

My bad! Double check I didn't clobber anything you added but I think I resolved the conflicts.

jamiew commented 3 years ago

Lgtm! Thanks for the cleanup

jamiew commented 3 years ago

Arg handling is kind of goofy; maybe it's OK to just ask everyone to use magic-animal-names with dashes? Certainly easier to support. I was using like hotspot add "large blue ostrich" (with quotes) but that is weird too

jamiew commented 3 years ago

One other ergonomic upgrade would be setting a default MAX if not present. But that only applies to oldies like me with out of date .env's

ThaMunsta commented 3 years ago

Yeah I think that's reasonable and would keep it cleaner. I don't like the arg joining but the only alternative I could think of was args[2] = message.content.replace(/hotspot activity /ig, '').replace(/ /g, '-'); and I can't say I like it any better. I also agree on the max. I ran into the same thing when moving the code to a "more production" environment and I forgot to update the .env - oops! Good stuff; this has been fun!