? ? Pending

User tests: Successful: Unsuccessful:

avatar dgrammatiko
dgrammatiko
30 Jul 2018

Summary of Changes

Since #21294 raised way too many questions and it's pretty obvious that people couldn't follow what was happening in there, here is a PR with less files touched.
So this PR is moving the assets of the templates to the source folder

  • naming and placement is debatable, eg /build/sources or /media-source or whatever
  • right now the command node build.js --compile-css will get us where we were before the PR. The problem is that to do so we're hardcoding the paths and that is why I proposed that the css files of the templates should be on a folder in the media folder (an exact copy of the media-source structure needs 0 hardcoding)

Testing Instructions

Fetch, pull, run node build.js --compile-css and check that templates (the css part, js is not covered here) is still intact

09144a2 28 Jul 2018 avatar dgrammatiko fixes
avatar dgrammatiko dgrammatiko - open - 30 Jul 2018
avatar dgrammatiko dgrammatiko - change - 30 Jul 2018
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 30 Jul 2018
Category Administration Templates (admin) JavaScript Repository
avatar mbabker
mbabker - comment - 30 Jul 2018

The problem is that to do so we're hardcoding the paths and that is why I proposed that the css files of the templates should be on a folder in the media folder

And the hardcoding of paths (for what I'm not clear exactly) is an issue why? Do realize a fair number of us don't only git clone this repo for contributing to core, we also work on our own projects using this local installation. Already running the build tools on 4.0 I have seen occasional warnings about some of the directories I have symlinked to other projects, our tool suite shouldn't be over eager here and be trying to process files it isn't designed to work with.

avatar dgrammatiko
dgrammatiko - comment - 30 Jul 2018

our tool suite shouldn't be over eager here and be trying to process files it isn't designed to work with

I can work around this, np. The whole idea, though, with the separation of source and distribution is something that we need asap because right now the repo is marking a bazillion of compiled/minified files which is extremely inconvenient for anyone that wants to contribute. We should care only for the sources all the compiled/transpiled and minified files shouldn't exist in the repo. I hope we all agree on that

avatar Bakual
Bakual - comment - 30 Jul 2018

Tried to test but got a syntax error:

SyntaxError: ./media/plg_system_stats/js/stats-message.es6.js: Unexpected token (5:9)

I would probably put the files into /build/media or /build/media-source/ to avoid having another root folder. This way it's also clear for anyone that this folder isn't in the distributed CMS.

Please remove the index.html files. I don't think there is a reason for those.

avatar dgrammatiko
dgrammatiko - comment - 30 Jul 2018

Please remove the index.html files. I don't think there is a reason for those.

I will once I have a step to create the missing folders per template.

I would probably put the files into /build/media or /build/media-source/

Also, I don't mind using the build folder. We already have plenty of code in there

avatar Bakual
Bakual - comment - 30 Jul 2018

I will once I have a step to create the missing folders per template.

Does JS not generate path automatically if you create a file?

I don't mind using the build folder. We already have plenty of code in there

Isn't that the purpose of that folder? To hold everything related to build a release? ?

avatar brianteeman
brianteeman - comment - 31 Jul 2018

Maybe it's just me but every time I see an acronym written in capital case it hurts my eyes

eg
compileJs ==> compileJS
compileCss ==>compileCSS
compileCe ==> compileCE

avatar Bakual
Bakual - comment - 31 Jul 2018

@dgrammatiko Can you take care of the syntax error so this can be tested?
And of course throw a bone to hound so he is satisified.

avatar dgrammatiko
dgrammatiko - comment - 31 Jul 2018

Maybe it's just me but every time I see an acronym written in capital case it hurts my eyes

@brianteeman this cannot be done the reason is that this line:

if (Program.compileJs) {

translates to --compile-js. I'm sorry for your eyes, but there is nothing I can do about it.

avatar dgrammatiko
dgrammatiko - comment - 31 Jul 2018

If there is an agreement on moving the assets to a common folder I'll redo this PR. For now separation of sources from the distributable media folder is way more important

avatar dgrammatiko dgrammatiko - change - 31 Jul 2018
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2018-07-31 15:45:37
Closed_By dgrammatiko
Labels Added: ? ?
avatar dgrammatiko dgrammatiko - close - 31 Jul 2018

Add a Comment

Login with GitHub to post a comment