Conversation
docker/Makefile
Outdated
build: static | ||
$(dc) build stb-python | ||
$(dc) build 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.
dc -> dcb
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.
build: static -> build: build-static
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.
TO-DO
docker/Makefile
Outdated
$(dc) up -d stb-python | ||
$(d) run --rm --volumes-from stb-python --workdir=/usr/app/src -it coala/base coala | ||
$(dc) up -d app | ||
$(d) run --rm --volumes-from app --workdir=/usr/app/src -it coala/base coala |
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 move/use the coala as service to docker-compose
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.
TO-DO
docker/Makefile
Outdated
$(dc) exec stb-python python manage.py migrate | ||
$(dc) exec app python manage.py migrate | ||
|
||
fixtures: |
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.
load-fixtures will be more clearly
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.
@artemiy312 ,
we have conflict with short vs clear commands.
I think commands should be short for the first. And clear for the second.
We type them much more often, then remember.
We all use Linux command ls
, but not show-files
.
New teammates will dig into Makefile anyway: with short or not commands.
So, i propose to leave short names of the commands, but add some doc for them in Makefile. We'll do it at fidals/shopelectro#303
&& python manage.py loaddata stroyprombeton/fixtures/dump.json \ | ||
" | ||
|
||
prices: |
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.
generate-prices will be more clearly
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.
@artemiy312 , let's leave prices
, because we already have django command with the same name
docker/Makefile
Outdated
$(MAKE) build-static | ||
# Create admin user with login/pass: admin/asdfjkl; | ||
# Launch "collectstatic" not in static recipe because ManifestStaticStorage writes to db | ||
$(dc) exec app bash -c "\ |
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.
Maybe we should reuse existing receipts instead of it, what do you think?
$(MAKE) migrate
$(MAKE) load-fixtures
$(MAKE) collectstatic
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.
TO-DO
docker/env_files/paths
Outdated
@@ -0,0 +1,5 @@ | |||
# Identify the dependencies folder |
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 must be ignored by git
docker/env_files/ports
Outdated
@@ -0,0 +1,6 @@ | |||
# Exposed ports |
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 must be ignored by git
docker/images/python/Dockerfile.dev
Outdated
FROM python:slim | ||
|
||
RUN apt-get update \ | ||
# wget is needed for working with ftp |
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.
We do not work with ftp
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.
TO-DO
docker/images/python/Dockerfile.dev
Outdated
# wget is needed for working with ftp | ||
# git is needed for pip | ||
&& apt-get install --no-install-recommends --no-install-suggests -y \ | ||
wget git make wkhtmltopdf xvfb gettext |
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.
seems it will be enoughwkhtmltopdf xvfb gettext
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.
@artemiy312 , git
and make
are required too
docker/images/python/Dockerfile.prod
Outdated
@@ -0,0 +1,8 @@ | |||
FROM fidals/se:dev |
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.
fidals/se:dev -> fidals/stb:dev
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.
TO-DO
c99b52b
to
3ad5d83
Compare
24fb7d5
to
854e71a
Compare
@artemiy312 , refactored drone.yml too. Pay attention to it plz |
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.
Few comment about Dockerfile for python
docker/Makefile
Outdated
# for serv env | ||
deploy-dev: | ||
$(MAKE) create-env | ||
# $(MAKE) create-config |
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.
Seems it should be uncommented
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 depends on the task. I'll add comment
TODO
docker/images/python/Dockerfile.dev
Outdated
RUN apt-get update \ | ||
# git is needed for pip | ||
&& apt-get install --no-install-recommends --no-install-suggests -y \ | ||
git make wkhtmltopdf xvfb gettext |
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.
We do not use git and make anywhere, so we should remove it.
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.
@artemiy312 ,
> # git is needed for pip
Removed it.
docker/images/python/Dockerfile.dev
Outdated
# git is needed for pip | ||
&& apt-get install --no-install-recommends --no-install-suggests -y \ | ||
git make wkhtmltopdf xvfb gettext | ||
&& apt-get remove --purge -y git \ |
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 line should be removed as above
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.
Done.
8376cbd
to
d8382bb
Compare
- Review#2 fix. Annotate commented string - Review#2 fix. Rm git install from dockerfile.dev - Fix forgotten env vars. Rm pdd task in favor of se#345 - Resurrect#1 gulp build - Launch tests #1 - Fix forgotten drone file - Review#1 fixes - Devops configs renew
d8382bb
to
fa9a58e
Compare
Closes #142