Enhancement/template generator #59
Conversation
| @@ -1,9 +0,0 @@ | |||
| language: node_js | |||
There was a problem hiding this comment.
I believe this change is being made in #58
| @@ -1,34 +1,37 @@ | |||
| # TechWeek Website [](https://travis-ci.org/redbrick/techweek.dcu.ie) | |||
| # TechWeek Website [](https://circleci.com/gh/redbrick/techweek.dcu.ie/tree/master) | |||
There was a problem hiding this comment.
Can you move it to another PR? This should be added in #58 but I didn't realize it at the time.
| function render (source, events, url) { | ||
| let template = Handlebars.compile(source); | ||
| let output = template(events); | ||
| if (!fs.existsSync(__dirname + '/../dist')) { fs.mkdirSync(__dirname + '/../dist'); } |
There was a problem hiding this comment.
Can you move fs.mkdirSync to a new line?
|
|
||
| function render (source, events, url) { | ||
| let template = Handlebars.compile(source); | ||
| let output = template(events); |
There was a problem hiding this comment.
I think inlining this would be neater as template its not used anywhere else.
const output = (Handlebars.compile(source))(events);
| const events = require(__dirname + '/../events.json'); | ||
| const template = fs.readFileSync(__dirname + '/../template.handlebars', "utf-8"); | ||
|
|
||
| function render (source, events, url) { |
There was a problem hiding this comment.
Can you add documentation to this function? What do source, events and url mean in this case.
| if (!fs.existsSync(__dirname + '/../dist')) { fs.mkdirSync(__dirname + '/../dist'); } | ||
| dir = __dirname + '/../dist'; | ||
| if (url) { | ||
| dir = dir + '/' + url; |
There was a problem hiding this comment.
Please use path.join.
| fs.mkdirSync(dir); | ||
| } | ||
| } | ||
| fs.writeFile(dir + '/index.html', output, 'utf8', (err) => { if (err) throw err; }); |
There was a problem hiding this comment.
Move if statement to a new line?
There was a problem hiding this comment.
considering its an error check id prefer it to be one line
| @@ -0,0 +1,3 @@ | |||
| machine: | |||
There was a problem hiding this comment.
This was added in #58. Please remove.
| "end": 7, | ||
| "month": "March", | ||
| "year": 2014, | ||
| "live": "2014-03-03T12:00:00", |
There was a problem hiding this comment.
Would Unix timestamp be more appropiate? I am not sure is this representation understandable by languages other than JS.
There was a problem hiding this comment.
id prefer the timestamp to be human readable
| host: 'techweek.dev', | ||
| livereload: true | ||
| }); | ||
| }); | ||
|
|
||
|
|
||
| gulp.task('fonts', function() { | ||
| return gulp.src('./node_modules/materialize-css/fonts/**') | ||
| .pipe(gulp.dest('dist/fonts')); | ||
| return gulp.src('./node_modules/materialize-css/fonts/**') |
There was a problem hiding this comment.
I should probably make a new issue for this, would moving to https://github.com/material-components/material-components-web be beneficial seeing as its the (yet another) official Material Design framework?
There was a problem hiding this comment.
create a new issue for that, would also be good to remove jquery as part of that
There was a problem hiding this comment.
issue #60 opened for this
| @@ -1,183 +0,0 @@ | |||
| [{ | |||
There was a problem hiding this comment.
I'm starting to think the json of the site should probably be in a separate repo since it adds a decent bloat to the diff
There was a problem hiding this comment.
yeah the json was originally never really meant to be part of the repo so moving it to a submoule would proabbly be a good idea
| @@ -0,0 +1,74 @@ | |||
| <!DOCTYPE html> | |||
close #21