chore(code_share): #59
Conversation
aniketmaithani
commented
Sep 28, 2017
aniketmaithani
commented
- project structure changes
- TODO : remove Hardcoded Values
- Code Cleanup
- project structure changes - TODO : remove Hardcoded Values - Code Cleanup
|
@sourabhtk37 review me please. :) If this makes sense to you we can go ahead and go ahead with the current structure. :) |
| @@ -26,7 +26,7 @@ def home(request): | |||
| """ | |||
|
|
|||
| if request.method == 'GET': | |||
| return render(request, 'app_code_share/homepage.html', {}) | |||
| return render(request, 'homepage.html', {}) | |||
There was a problem hiding this comment.
It would be better to have app_code_share/<template_name> format I suppose since we are having folder structure as templates/<app_name>/<template_name>
There was a problem hiding this comment.
so technically code_share is your main app_name_folder/templates :) and the rest of the things such as wsgi etc. remains outside hence I have kept it to this. Also I have made changes in settings.settings in STATIC_DIRS part to accomodate the change I've done :)
It would be good to have all static files in one place :)
There was a problem hiding this comment.
your take on this one? @sourabhtk37
There was a problem hiding this comment.
Yeah, the changes you made are good, global template file with sub folders and configuration related files in root dir and all.
What I requested to be changed is this:
'DIRS': ['code_share/templates/app_code_share', 'code_share/templates/error', ],
If we changed to having 'code_share/templates/' only, then writing in views something like return render(request, 'homepage.html', {}) would become return render(request, 'app_code_share/homepage.html', {}) which is better to find the file. Just my opinion since in case we have many sub folders it becomes quite hard for new contributors to find the file.
There was a problem hiding this comment.
Hmmm.. makes sense. Let me go through it. Will it push it by tonight :)
| @@ -0,0 +1,29 @@ | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
Migrations shouldn't be here. Please add migration folders to .gitignore file.
There was a problem hiding this comment.
any specific reason? for adding migrations folder to .gitignore
There was a problem hiding this comment.
FYI
The migration files for each app live in a “migrations” directory inside of that app, and are designed to be committed to, and distributed as part of, its codebase. You should be making them once on your development machine and then running the same migrations on your colleagues’ machines, your staging machines, and eventually your production machines.
There was a problem hiding this comment.
I stand corrected for this. Read more here as to why one should commit migrations.
