Skip to content

!question & !poll Commands#166

Merged
brennondenny merged 5 commits intoRelease3from
r3-feature-poll
May 22, 2018
Merged

!question & !poll Commands#166
brennondenny merged 5 commits intoRelease3from
r3-feature-poll

Conversation

@brennondenny
Copy link
Contributor

This feature adds in the !question and !poll commands to Max Bot. Once the bot is up and running locally type !help for information on how to use the commands.

@mpeck99
Copy link

mpeck99 commented May 22, 2018

I have tested your bot locally and it works perfectly! Good Job!

Copy link

@mattpezzente mattpezzente left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested aren't critical to project functionality, but should be attempted as time allows before merging.

const { message } = this;
const question = message.parsed[1];
const questionSpaced = question.split('-').join(' ');
const content = message.content;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm picking up a linting issue here, even though it seems to have gone through the tests successfully. However, I do recommend de-structuring it:

const { content } = messsage;

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure why you're getting that linting issue, I don't see it and like you said it does pass the CircleCI test. I'm using content to convert the message.content into a usable string, but still need to use const {message} = message} so I get the initial information from Discord.

options += `\r ️\u0039\u20E3${message.parsed[i]}`;
}
}
} else {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a space between the special characters and the poll options for minor readability improvement

} else if (i === 10) {
message.react('\u0039\u20E3');
}
}

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I'm reading this correctly, I believe that this can be improved without doing multiple checks? For example:

message.react(`\u003${i}\u20E3`);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this is a better way to solve this, the Style Guide restricts incomplete unicode like \u003${i}.

@BHess2653
Copy link

Poll and Question worked for me and I didn't see any issues.

@brennondenny brennondenny merged commit b24077f into Release3 May 22, 2018
brennondenny added a commit that referenced this pull request May 22, 2018
brennondenny added a commit that referenced this pull request May 22, 2018
@reactivepixel reactivepixel deleted the r3-feature-poll branch August 21, 2019 13:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants