Add front end build to container #73
Conversation
| @@ -6,7 +6,7 @@ | |||
| "scripts": { | |||
| "build": "webpack", | |||
| "lint": "tslint -c tslint.json 'richie/js/**/*.ts'", | |||
| "sass": "node-sass richie/static/scss/_main.scss data/static/css/main.css", | |||
| "sass": "node-sass richie/static/scss/_main.scss richie/static/css/main.css", | |||
There was a problem hiding this comment.
@mbenadda I was wondering why we didn't put the result of the css build to richie/build instead of richie/static as it is the one in gitignore?
There was a problem hiding this comment.
I think richie/static/css is the right place for a css build, and, IMO, richie/build as no particular reason to exists, it should be removed and the build target should be richie/static/js... but that's another discussion I guess.
There was a problem hiding this comment.
Ok I can explain and then @mbenadda will need to add these explanations in the documentation because these design decisions are not obvious it's true.
The reason we need a /build folder separate from the /static folder is that the static folder is committed in the repo. The result of the build is not committed. So when you work on your project, you have a watch which rebuilds your css/js on which modification and you will easily make a mistake and commit the file results if they are not in a separate folder.
We could have made a specific folder inside /static folder but then:
- this difference would transpire in our collected static folder which is not what we want,
- it is clearer to have a /build directory and a best practice in frontend development,
- mixing committed static files and build results is what is done in Open edX and I found it very confusing.
We then add the /build directory in the template finder so it is collected.
There was a problem hiding this comment.
If you add builds target directories (static/css + static/js) to your .gitignore, unless you really want it, you cannot commit the builds, and this is way more simpler than adding another static directory.
Plus, IMO, the richie/static/scss directory shouldn't be here, but rather in richie/assets/scss along with the js sources. I would find it way more cleaner/simpler this way. But that's only my opinion/habit.
There was a problem hiding this comment.
I've opened a new issue to pursue this discussion, as it's off-topic here: see #74
There was a problem hiding this comment.
static/css + static/js to your .gitignore -> ok it's the same to me as what i proposed. So let's do it now because the css in static/css without gitignore like we have now is not good.
Styles build should be stored in the main static directory and then copied via collectstatics where it will get served.
jmaupetit
force-pushed
the
add-front-end-build-to-container
branch
from
7c8ea72
to
299aa73
Compare
jmaupetit
changed the title
| @@ -16,6 +16,7 @@ __pycache__/ | |||
| # Site media | |||
| /data/ | |||
| richie/build | |||
| richie/static/css | |||
|
|
|||
There was a problem hiding this comment.
then add also static/js I'm ok with this.
There was a problem hiding this comment.
👌 I think I'll do this in a new PR to address our conclusions from #74
Front-end JS and CSS builds were missing in Richie's container. This is now fixed.