-
Notifications
You must be signed in to change notification settings - Fork 3
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
Update dependencies and add env variables to get repo working on Heroku #24
Conversation
prototriangle
commented
Oct 27, 2018
•
edited
Loading
edited
- Edits were tested on Heroku.
- Used a newly created MyMLH application for testing.
- Updated scheme of mlh sign-in callback to use https (perhaps this should be removed)
(Probably should squash) |
Once merged, I'll set up a Heroku app that will automatically update from |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like good things are happening here. The main thing I want to see changed is that the links point to the canon repo instead of your fork. Good work 👍
@@ -1,6 +1,6 @@ | |||
# Cog | |||
|
|||
[![Deploy](https://www.herokucdn.com/deploy/button.svg)](https://heroku.com/deploy?template=https://github.com/CreatEDHack/cog) | |||
[![Deploy](https://www.herokucdn.com/deploy/button.svg)](https://heroku.com/deploy?template=https://github.com/prototriangle/cog) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should probably stay as CreatEDHack if this is being merged into CreatEDHack
@@ -38,7 +38,7 @@ an additional account. | |||
The easiest way to deploy Cog is to smash this Deploy to Heroku button right | |||
here: | |||
|
|||
[![Deploy](https://www.herokucdn.com/deploy/button.svg)](https://heroku.com/deploy?template=https://github.com/techx/cog) | |||
[![Deploy](https://www.herokucdn.com/deploy/button.svg)](https://heroku.com/deploy?template=https://github.com/prototriangle/cog) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above
@@ -1,8 +1,14 @@ | |||
{ | |||
"name": "Cog", | |||
"description": "A hackathon hardware check-out system by HackMIT", | |||
"repository": "https://github.com/techx/cog", | |||
"repository": "https://github.com/prototriangle/cog", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
^
"env": { | ||
"OAUTH_ID": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember these were environment variables before. What is the behaviour of the app now that these are in app.json? What does this change? Are the docs updated?
@@ -9,7 +9,7 @@ cffi==1.10.0 | |||
chardet==3.0.4 | |||
click==6.7 | |||
configparser==3.5.0 | |||
cryptography==1.7.2 | |||
cryptography==2.3.1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be nice to see a test (or just a reasoned argument) that confirmed this change didn't break the app
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, linking in #22