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

jcassidy #133

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

Conversation

cassidyjamesw
Copy link

No description provided.

Copy link

@TheDeterminator TheDeterminator left a comment

Choose a reason for hiding this comment

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

James, this code looks great! Good job getting through this assignment. I assume based off of your last commit message that you also had trouble with the "old_auth tables" bug that plagued so many people? Whatever the case good job getting through any blockers you may have faced while doing this code. By way suggestion you could add a few more comments to your code (although I appreciate taht most of it was prewritten for this assignment). And you should name our PRs: [FirstName LastName] - Name-Of-Project. I don't know if your PM has ever said explicitly but it will make it easier for him to find your pull requests. All the same, great job!

@@ -26,7 +28,7 @@
# SECURITY WARNING: don't run with debug turned on in production!
DEBUG = config('DEBUG', cast=bool)

ALLOWED_HOSTS = []
ALLOWED_HOSTS = os.environ.get("ALLOWED_HOSTS").split(",")

Choose a reason for hiding this comment

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

This is interesting, you could also have casted to a csv() type to get similar functionality.

saidmessage = data['saidmessage']
room = player.room()
currentPlayerUUIDs = room.playerUUIDs(player_id)
for p_uuid in currentPlayerUUIDs:

Choose a reason for hiding this comment

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

This is good, have you thought about the case when a user tries to break your functionality by sending an empty message? If you really wanted to stretch you could think about how other types of input based attacks might be handled by this code but that is beyond the scope of this project.

@@ -1,3 +1,5 @@
pip freeze > requirements.txt

Choose a reason for hiding this comment

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

When did you run pip freeze? I thought there might be more modules in this file but maybe not.

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