New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
issue/1683+issue/1678: grunt allow .txt/.json, include build model in output #1684
Conversation
grunt/tasks/build-config.js
Outdated
var hideAttributes = [ 'outputdir', 'sourcedir', 'root' ]; | ||
hideAttributes.forEach(function(attrName) { delete buildConfig[attrName]; }); | ||
|
||
grunt.file.write(buildConfigPath, JSON.stringify(buildConfig, null, " ")); |
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.
Could you change the third argument of JSON.stringify
to an integer, as per #1462 (comment)?
grunt/tasks/schema-defaults.js
Outdated
@@ -57,7 +57,7 @@ module.exports = function(grunt) { | |||
//iterate through lanugage folders | |||
grunt.file.expand({filter: 'isDirectory'}, grunt.config('outputdir') + 'course/*').forEach(function(path) { | |||
var currentCourseFolder = path; | |||
var currentCourseJsonFile = currentCourseFolder + '/' + 'course.json'; | |||
var currentCourseJsonFile = currentCourseFolder + '/' + 'course.' + grunt.config('jsonext'); |
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.
Should we be caching jsonext
outside the forEach
loop like here?
@@ -15,7 +15,7 @@ module.exports = function (grunt) { | |||
"blocks", | |||
"components" | |||
].forEach(function (filename) { | |||
var src = path.join(srcPath, "course", targetLang, filename+".json"); | |||
var src = path.join(srcPath, "course", targetLang, filename+"."+grunt.config("jsonext")); |
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.
Should we be caching jsonext
outside the forEach
loop like here?
src/core/js/models/buildModel.js
Outdated
|
||
}); | ||
|
||
return BuildModel |
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.
;
missing from end of line.
@oliverfoster you have a conflict on grunt/helpers.js |
ta |
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.
Is there any scope for the grunt task converting the (src) JSON to (build) text for you, or is that just crazy talk?
yea, totally could do. you mean changing the file name extensions in the output directory? update: this gets really irritating when the src and output paths are the same thing |
I think the setting of |
I would normally concur @brian-learningpool but I'd like this API to be consistently available at run-time as it'll be useful for passing compile-time variables into the run-time environment. Is it a size concern regarding the bower.json from all of plugins? |
It's a combination of the size and the fact the entire bower.json file is dropped in. This could reveal infrastructure details about the location of non-core or non-OS plugins, i.e. through the A lot of the information in the Build as it currently is is non-essential to assist with debugging, e.g. |
cool, if i pick out the necessaries and drop the others would that suffice? bower.json {
"name": "adapt-contrib-vanilla",
"version": "2.1.1",
"framework": "^2.1.0",
"displayName": "Vanilla",
"theme": "vanilla",
"description": "A core bundled theme",
"main": "js/vanilla.js",
"keywords": [
"adapt-plugin",
"adapt-theme"
],
"license": "GPLv3"
} which is: package.json{
"name": "adapt_framework",
"version": "2.1.3",
"description": "Adapt Learning output framework",
"repository": {
"type": "git",
"url": "git://github.com/adaptlearning/adapt_framework.git"
},
"license": "GPL-3.0"
} which is: |
Works for me -- thanks. |
grunt/config/replace.js
Outdated
|
||
var jsonext = grunt.config('jsonext'); | ||
var pathToConfig = path.join(courseDir, 'config.'+jsonext); | ||
var courseDir = path.join(options.outputdir, 'course'); |
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.
switched to always use the build folder files as they're always available and it reduces the code here
|
||
var generatePatterns = function() { | ||
var jsonext = grunt.config('jsonext'); | ||
var pathToConfig = path.join(courseDir, 'config.'+jsonext); | ||
|
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.
added these in here, as the config isn't ready until this point
|
||
// Ensure that only whitelisted attributes can be replaced. | ||
courseJson = _.pick(courseJson, 'title', 'displayTitle', 'body', 'description'); | ||
configJson = _.pick(configJson, '_xapi', '_spoor'); |
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.
removed these restrictions as both files are distributed with the finished product and it'll allow more flexibility in the string replacement
#1798
grunt/tasks/build-config.js
Outdated
buildConfig.plugins.push(plugin); | ||
}); | ||
|
||
// remove path specific variables | ||
var hideAttributes = [ 'outputdir', 'sourcedir', 'root' ]; | ||
hideAttributes.forEach(function(attrName) { delete buildConfig[attrName]; }); | ||
|
||
grunt.file.write(buildConfigPath, JSON.stringify(buildConfig, null, 2)); | ||
grunt.file.write(buildConfigPath, JSON.stringify(buildConfig)); |
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.
write adapt/js/build.min.js
minified to reduce file size
issue/1678:
course/**/*.json
files to becourse/**/*.txt
or any other extension in both grunt and the framework.to test:
config.json
,en/course.json
,en/contentObject.json
,en/articles.json
,en/blocks.json
anden/components.json
toconfig.txt
,en/course.txt
,en/contentObject.txt
,en/articles.txt
,en/blocks.txt
anden/components.txt
.$ grunt dev --jsonext=txt
issue/1683:
grunt dev
orgrunt build
You should have an object which looks something like: