bmbrands / theme_bootstrap

A Moodle theme based on the Bootstrap CSS framework
113 stars 112 forks source link

Remove local grunt amd task #424

Closed jobyh closed 8 years ago

jobyh commented 8 years ago

When using grunt to build amd modules inside the theme grunt picks up the local theme Gruntfile rather than the core gruntfile which contains the amd task config and previously would fail as a result.

To work around this a theme-level amd task was added to duplicate the core functionality. Its possible to work around this issue by explicitly passing the gruntfile grunt option when calling grunt e.g.

% cd theme/bootstrap/amd
% grunt --gruntfile="../../../Gruntfile.js"

Alternatively @davidscotson pointed out a grunt plugin which can be used to load tasks from another Gruntfile behind the scenes and achieve the same without needing to pass --gruntfile.

If we use one of these methods the duplicated code can be removed / will no longer need maintaining.

bmbrands commented 8 years ago

Thanks for looking into this Joby! I did not know of the --gruntfile option. And even if it should have been easy enough to figure out. Before I simple moved the Gruntfile.js somewhere else and compile the amd using Moodle's Grunt because I was lazy and wanted to continue my work instead of mind-switching to Grunt stuff.

So for me the ideal solution would have been just to have the command grunt adm tell me "Your doing it Wrong. Go ask Moodle's Gruntfile to to compile your amd by typing "grunt --gruntfile="../../../Gruntfile.js. My work is done here." :)

gjb2048 commented 8 years ago

Oh, just thought, what if the theme is installed in $CFG->themedir ?

gjb2048 commented 8 years ago

Actually I don't think the original coped either. Grrr.

jobyh commented 8 years ago

I think that could be dealt with in the warning example by making clear it's a case of the developer specifying the path to the core Gruntfile in their development environment. e.g.Use grunt --gruntfile="/path/to/your/dirroot/Gruntfile.js"

jobyh commented 8 years ago

OK great - I'll put something together based on the above and reference the pull request here when done.