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

Add automatch capability #95

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

Add automatch capability #95

wants to merge 13 commits into from

Conversation

amikula
Copy link

@amikula amikula commented Feb 20, 2016

For doubles games, you have to wait for enough players to join and you also have to come up with a fair match. The automatch command takes care of both. By default, automatch will be on for 5 minutes, but users can specify an end time or a duration. See README.md for details.

@dblock
Copy link
Owner

dblock commented Feb 20, 2016

First, great to see a complete feature PRed! Excited to get this merged.

The things I like is that it just automatically matches players. But I don't love the fact that it forces this to be a doubles game or that the language doesn't match challenge. What do you think of renaming automatch? Maybe play anyone in the next 10 minutes and play doubles in the next 10 minutes?

What do you think?

Plus:

  • Fix the build, the spec failure looks legit.
  • Squash your commits.

I'll make some comments inline.

@@ -1,5 +1,6 @@
### Changelog

* Added automatch support for doubles matches - [@amikula](https://github.com/amikula).
Copy link
Owner

Choose a reason for hiding this comment

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

Link the PR, like below.

@amikula
Copy link
Author

amikula commented Mar 1, 2016

Thanks for all the great comments! I'll work on it...really looking forward to getting this in.

Unfortunately, this past weekend when I loaded the code onto a different computer, I can't get the test suite to run successfully. Looks like DatabaseCleaner isn't working properly for some reason I can't ascertain. I guess that will have to be a different pull request once I get to the bottom of it. :-P

@dblock
Copy link
Owner

dblock commented Mar 2, 2016

Happy to help! Post your detailed issues here.

@dblock
Copy link
Owner

dblock commented Apr 27, 2016

Bump @amikula, I believe your DatabaseCleaner issue is fixed, they released a newer version that works with newer MongoDBs.

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.

2 participants