sfbrigade / datasci-sba

Solving problems with the Small Business Administration
10 stars 18 forks source link

Django Example #35

Closed VincentLa14 closed 7 years ago

VincentLa14 commented 7 years ago

1. Brief Summary of what this PR accomplishes (140 characters or less. If you find trouble describing what you are doing in this length, consider breaking the PR into multiple ones.)

Adds Django backend for building out the webserver and writing first Django app

2. Link to Trello Ticket

https://trello.com/c/E2bEUTip/48-build-first-django-app-and-webserver

3. More detailed description and other questions to address in code review

  1. Building Django Webserver which includes first Django app with models of our Database
  2. Initial front end map using Google Maps API

To Do: Testing (Perhaps in a separate PR)?

4. Remember to tag reviewers!

VincentLa14 commented 7 years ago

@avdonovan thanks for all of this! I'm going to dive in now to see if I can make sense of this. My feeling is that if we're in a semi-wrapped state, we can just merge this to master first? Getting to the point where the PR is big enough that it's hard to review everything.

Also feel free to edit the PR description; I don't think I'm using the technically right language.

VincentLa14 commented 7 years ago

Ok I looked through this, and I think at least superficially, this makes sense. I have a few questions which I'll ask in a separate comment, but any problems just merging this to master for now once we resolve those questions? For anyone else wondering how to rerun this, you can do

python manage.py runserver

This gives me

(datasci-sba) VincentLa@ch-C02Q6HPBG8WN first_project (django-tutorial) $ python manage.py runserver
Performing system checks...

System check identified no issues (0 silenced).
August 22, 2017 - 02:19:49
Django version 1.11.3, using settings 'first_project.settings'
Starting development server at http://127.0.0.1:8000/

And then go to http://127.0.0.1:8000/app in your browser

avdonovan commented 7 years ago

@VincentLa14 I'm glad to hear this branch makes sense. Lemme know if you have other questions too, and/or we can go over it on Wednesday. I agree it's pretty close to merging. There are a few more things I wanted to do first; I'll try to get to these tonight:

  1. Add a readme for how to run locally (I'll just copy your comment for this)
  2. Better naming of the apps to distinguish what they do; e.g. rename first_project/firstapp to first_project/api_server and first_project/sba_app to first_project/web_app
  3. Resolve the duplicate models per your comment
avdonovan commented 7 years ago

OK I pushed 3 changes per my last comment, lemme know how they look.

VincentLa14 commented 7 years ago

@avdonovan LGTM I have some questions also just related to still being new to Django. I can ask you about it later tonight. But I'm good to merge this in whenever at this point. Thanks for doing the renaming.