JATaber twitch notification#156
Conversation
Changing from the automated way of annoucing when a user goes live to a simple command way of doing this feature
JATaber
assigned
JATaber and unassigned
reactivepixel,
BHess2653,
brennondenny,
mattpezzente and
mpeck99
mattpezzente
left a comment
mattpezzente
left a comment
There was a problem hiding this comment.
Overall a very good tool. I like your use of ES6 throughout your solution. Make sure to comment your code as well, and take a look at the reviews I've left for your consideration.
bot/controllers/twitchLive.js
Outdated
| const util = require('apex-util'); | ||
| // const util = require('apex-util'); | ||
|
|
||
| class StreamerLiveController extends BaseController { |
There was a problem hiding this comment.
Would it be more appropriate to name the class in tangent with the file name? Example: TwitchLiveController
There was a problem hiding this comment.
I named this class with other streaming services in mind. That way a command for Mixer and others can be added at a later point.
bot/controllers/twitchLive.js
Outdated
| @@ -19,7 +19,10 @@ class StreamerLiveController extends BaseController { | |||
| addTwitchAction() { | |||
There was a problem hiding this comment.
Would naming this to something like "announceTwitchLive" be more appropriate?
bot/controllers/twitchLive.js
Outdated
| '!twitchLive <twitch_name>', | ||
| 'Twitch Live', | ||
| 'A quick easy way to ', | ||
| 'A quick easy way to announce you are going live on Twitch.', |
There was a problem hiding this comment.
Grammar suggestion:
'A quick easy way...'
vs.
'A quick and easy way...'
bot/controllers/twitchLive.js
Outdated
| const streamMessage = `Hey everyone! ${message.author.username} is going live playing ${twitchGame}!! Check him out at ${twitchURL} !`; | ||
| const targetChannel = message.guild.channels.get('319548128267206666'); | ||
| const sender = message.author.username; | ||
| return targetChannel.send('```' + sender + ' has an announcment: ```' + streamMessage); |
There was a problem hiding this comment.
App crashes because I do not have the specific channel ID for testing. Make a provision to handle if the channel exists or not, maybe by sending the message to the default channel if the one you are searching for can't be found.
There was a problem hiding this comment.
Definitely, need to make sure the channel ID is changed appropriately so that it will in any channel not just the one with that specific ID.
There was a problem hiding this comment.
changed this to look for a specific channel name
bot/controllers/twitchLive.js
Outdated
| const controller = this; | ||
| this.commands = [ | ||
| new Command( | ||
| '!twitchLive', |
There was a problem hiding this comment.
Since your goal is to make the StreamerLiveController generic, consider changing the command too!live and add a service attribute, for example !live twitch myname mygame.
There was a problem hiding this comment.
I am going to implement this method making one command good for twitch or mixer
bot/controllers/twitchLive.js
Outdated
| ), | ||
| ]; | ||
| } | ||
| addTwitchAction() { |
There was a problem hiding this comment.
Consider changing this to "streamerLiveAction" to better fit the overall functionality.
bot/controllers/twitchLive.js
Outdated
| ), | ||
| ]; | ||
| } | ||
| addTwitchAction() { |
There was a problem hiding this comment.
Consider changing this to "streamerLiveAction" to better fit the overall functionality.
bot/client.js
Outdated
|
|
||
| // TODO: Set Twitch notification | ||
|
|
||
| // Message for new users |
There was a problem hiding this comment.
Remove the old Welcome Feature, this should only include code for the live stream notification.
There was a problem hiding this comment.
Thank you for this I am removing it now
6 participants
This is the feature to where a user can easily announce they are going live on Twitch.