Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

added a function to improve conversations with the bot around selecti… #9

Merged
merged 4 commits into from
Aug 18, 2016

Conversation

CiaranHannigan
Copy link
Contributor

…ng options from a numbered list.

Tests were added, and the resulting regex were tested in regex101.com

@coveralls
Copy link

Coverage Status

Coverage increased (+1.6%) to 84.444% when pulling 734ae81 on numbered_list_regex_improvement into b01f230 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.6%) to 84.444% when pulling 56d5ec3 on numbered_list_regex_improvement into b01f230 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+3.7%) to 86.517% when pulling a6b5a63 on numbered_list_regex_improvement into b01f230 on master.


// check < 1 or > 99
if (maxNum < 1 || maxNum > 99) {
return;
Copy link

@clanzen clanzen Aug 18, 2016

Choose a reason for hiding this comment

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

I wondered if maybe an exception should be thrown, but maybe not. The key will be how the calling code handles the response. Looks like it would need to check for undefined. If that's the case, maybe make a comment to that affect in the function header comments. That way if/when someone goes to use this in the future, there will be a reminder of what to expect.

@coveralls
Copy link

Coverage Status

Coverage increased (+3.7%) to 86.517% when pulling 99bf1ca on numbered_list_regex_improvement into b01f230 on master.

@clanzen clanzen merged commit f3b79d2 into master Aug 18, 2016
@clanzen
Copy link

clanzen commented Aug 18, 2016

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants