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

Avoids duplicates when the configured number of people to call out is… #29

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

timmytofu
Copy link
Contributor

… greater than the number active in the channel

… greater than the number active in the channel
@traceyvanp
Copy link

Hi! After I edited the slackbotExercise file to incorporate your pull request of avoiding duplicates, I'm getting an "Insecure Platform Warning" (InsecurePlatformWarning: A true SSLContext object is not available. This prevents urllib3 from configuring SSL appropriately and may cause certain SSL connections to fail.) The workout was working before, any ideas what could cause this error? It runs fine up to the point of announcing the next lottery, and then never announces.
Also, could I add another exercise "sit-ups", just by editing config.json (default.json) like this:
{
"id": 4,
"name": "calf raises",
"minReps": 20,
"maxReps": 30,
"units": "rep"
},
{
"id": 5,
"name": "sit-ups",
"minReps": 30,
"maxReps": 50,
"units": "rep"
}

@timmytofu
Copy link
Contributor Author

Not sure, I was seeing that warning all along…

@traceyvanp
Copy link

I ran the code using a later version of Python (2.5 to 2.9) and it works with no Insecure Platform Warning error!

@alexkolson
Copy link

@timmytofu Does this actually work? I merged it into my fork downstream and I get an index out of range error.

@@ -97,7 +99,7 @@ def selectUser(bot, exercise):
# User should be active and not have done exercise yet
if user in active_users and not user.hasDoneExercise(exercise):
bot.user_queue.remove(user)

Choose a reason for hiding this comment

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

Can we still be removing users as we are iterating? Won't this result in an Index Out of Range error now since we're not returning the user out of the loop?

…ember why I didn't do this in the first place
@timmytofu
Copy link
Contributor Author

Indeed you're right. I've updated it to use a for each loop, though I feel like there was a reason I didn't do that in the first place. Honestly I haven't done python in ten years and didn't do much then, so all of this is probably not the best.

@alexkolson
Copy link

@timmytofu I'm no python guru myself, and I really just asked to make sure I wasn't crazy because I think this feature is awesome and I want to pull it into my downstream fork. 👍

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.

3 participants