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

fixed up user_profile.py #6

Merged
merged 7 commits into from
Jul 25, 2020
Merged

fixed up user_profile.py #6

merged 7 commits into from
Jul 25, 2020

Conversation

JasonCheng1
Copy link
Collaborator

Let me know if this is what you want for user_profile.py

@JasonCheng1 JasonCheng1 requested a review from anitejb July 20, 2020 03:51
"python.linting.pylintEnabled": false,
"python.linting.flake8Enabled": true,
"python.linting.pylamaEnabled": false
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll add settings.json to .gitignore

I'm using something called black for linting

Copy link
Member

Choose a reason for hiding this comment

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

black is one way of doing it, we can also look into some other options like pylint. We should figure out which linting tool will be the best for our needs.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

im using black right now but lmk if you find anything better

app/config.py Outdated
DIRECTOR_CREDENTIALS = {
"email": "",
"password": ""
}


# uri should contain auth and default database(database name)
DB_URI = ""
DB_URI = "mongodb://127.0.0.1:27017/hackrd"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I can make a .gitignore for this page so we don't expose any credentials.

Copy link
Member

Choose a reason for hiding this comment

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

We should implement a config.example.py like LCS, so that each developer stores their own copy of the config.py file locally, and the repo only maintains a template file.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can we remove config.py so that github doesn't track it

Copy link
Member

@anitejb anitejb Jul 21, 2020

Choose a reason for hiding this comment

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

Yup let's remove this file, you already added in config.example.py so we're good to go!

app/config.py Show resolved Hide resolved
app/schemas.py Outdated Show resolved Hide resolved
app/user_profile.py Show resolved Hide resolved
app/user_profile.py Show resolved Hide resolved
app/views.py Outdated Show resolved Hide resolved
app/config.py Outdated
DIRECTOR_CREDENTIALS = {
"email": "",
"password": ""
}


# uri should contain auth and default database(database name)
DB_URI = ""
DB_URI = "mongodb://127.0.0.1:27017/hackrd"
Copy link
Member

Choose a reason for hiding this comment

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

We should implement a config.example.py like LCS, so that each developer stores their own copy of the config.py file locally, and the repo only maintains a template file.

Copy link
Member

@anitejb anitejb left a comment

Choose a reason for hiding this comment

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

Overall great work! I touched upon a few small things, but this is a great step for user_profile.py. You can go ahead and start working on some of the other files as well, and add those changes to this PR and we can merge it all in together.

app/schemas.py Outdated Show resolved Hide resolved
app/config.py Outdated
DIRECTOR_CREDENTIALS = {
"email": "",
"password": ""
}


# uri should contain auth and default database(database name)
DB_URI = ""
DB_URI = "mongodb://127.0.0.1:27017/hackrd"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Can we remove config.py so that github doesn't track it

@JasonCheng1 JasonCheng1 requested a review from anitejb July 21, 2020 04:12
Copy link
Member

@anitejb anitejb left a comment

Choose a reason for hiding this comment

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

Only thing left is to remove config.py, everything else looks good to me.

app/config.py Outdated
DIRECTOR_CREDENTIALS = {
"email": "",
"password": ""
}


# uri should contain auth and default database(database name)
DB_URI = ""
DB_URI = "mongodb://127.0.0.1:27017/hackrd"
Copy link
Member

@anitejb anitejb Jul 21, 2020

Choose a reason for hiding this comment

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

Yup let's remove this file, you already added in config.example.py so we're good to go!

team_size = len(coll("teams").find_one({"_id": team_name})['members'])
team_name = coll("teams").find_one({"members": {"$all": [email]}}, {"_id"})[
"_id"
]
Copy link
Member

Choose a reason for hiding this comment

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

black has some weird formatting lol

Copy link
Member

Choose a reason for hiding this comment

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

(you don't actually need to do anything here, just a comment)

@anitejb anitejb changed the base branch from master to develop July 21, 2020 13:02
@anitejb anitejb merged commit 4c84904 into develop Jul 25, 2020
@anitejb anitejb deleted the Jason branch July 25, 2020 16:31
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

2 participants